security issues with asterisk 1.2.10

[Tzafrir Cohen]

On Sun, Aug 27, 2006 at 04:23:00AM +0300, Tzafrir Cohen wrote:
> Update:
> 
> On Fri, Aug 25, 2006 at 12:50:54PM +0300, Tzafrir Cohen wrote:
> > Hi
> > 
> > I'm trying to figure out 
> > http://labs.musecurity.com/advisories/MU-200608-01.txt

> > 2. A format string issue with Record(). Probably in Sarge as well. Not
> > in the default configuration.
> 
> Seems to be purely a configuration issue. No code change involved. The
> only format interpreted in the filename is a harmless %d.

Update: after the release of Asterisk 1.2.11 a format string issue at
exactly the same place was actually spotted there:

  http://bugs.digium.com/view.php?id=7811

I have already commited a patch to the Sid package. Attached a dpatch
for the 1.0 Sarge package (it does not seem to be maintained in
pkg-voip).

-- 
Tzafrir Cohen         sip:tzafrir at local.xorcom.com
icq#16849755          iax:tzafrir at local.xorcom.com
+972-50-7952406          jabber:tzafrir at jabber.org
tzafrir.cohen at xorcom.com     http://www.xorcom.com
-------------- next part --------------
#! /bin/sh /usr/share/dpatch/dpatch-run
## apprecord_sprintf.dpatch by Tzafrir Cohen <tzafrir.cohen at xorcom.com>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: Fix a format string issue in app_record.c . 
## DP: See http://bugs.digium.com/view.php?id=7811 . 

@DPATCH@
diff -urNad asterisk-1.0.11.dfsg.1/apps/app_record.c /tmp/dpep.x11dA9/asterisk-1.0.11.dfsg.1/apps/app_record.c
--- asterisk-1.0.11.dfsg.1/apps/app_record.c	2005-11-29 20:24:39.000000000 +0200
+++ /tmp/dpep.x11dA9/asterisk-1.0.11.dfsg.1/apps/app_record.c	2006-08-31 00:23:06.885405446 +0300
@@ -44,6 +44,33 @@
 "User can press '#' to terminate the recording and continue to the next priority.\n\n"
 "Returns -1 when the user hangs up.\n";
 
+static char *filename_add_count(const char *fn, int count)
+{
+	char *realname;
+	char *tmp;
+	char cnt[32];
+	int can_subst = 1;
+
+	snprintf(cnt, sizeof(cnt), "%d", count);
+	tmp = realname = malloc(strlen(fn) + strlen(cnt) + 1);
+
+	while( *fn )
+	{
+		if (*fn == '%' && can_subst && fn[1] == 'd') {
+				strcpy(tmp, cnt);
+				tmp+=strlen(tmp);
+				can_subst = 0;
+				fn++;
+		} else {
+			*tmp = *fn;
+			tmp++;
+		}
+		fn++;
+	}
+	return realname;
+}
+
+
 STANDARD_LOCAL_USER;
 
 LOCAL_USER_DECL;
@@ -54,7 +82,7 @@
 	int count = 0;
 	int percentflag = 0;
 	char fil[256];
-	char tmp[256];
+ 	char *realfilename = NULL;
 	char ext[10];
 	char *vdata;
 	int i = 0;
@@ -139,6 +167,7 @@
 		if (silence > 0)
 			silence *= 1000;
 	}
+	free(realfilename);
 
 	if (vdata[i] == '|')
 		i++;
@@ -172,13 +201,17 @@
 	/* these are to allow the use of the %d in the config file for a wild card of sort to
 	  create a new file with the inputed name scheme */
 	if (percentflag) {
-		do {
-			snprintf(tmp, sizeof(tmp), fil, count);
+ 		realfilename = filename_add_count(fil, count);
+ 		count++;
+ 		while ( ast_fileexists(realfilename, ext, chan->language) != -1 )
+ 		{
+ 			free(realfilename);
+ 			realfilename = filename_add_count(fil, count);
 			count++;
-		} while ( ast_fileexists(tmp, ext, chan->language) != -1 );
-		pbx_builtin_setvar_helper(chan, "RECORDED_FILE", tmp);
+ 		}
+ 		pbx_builtin_setvar_helper(chan, "RECORDED_FILE", realfilename);
 	} else
-		strncpy(tmp, fil, sizeof(tmp)-1);
+ 		realfilename = strdup(fil);
 	/* end of routine mentioned */
 
 	LOCAL_USER_ADD(u);
@@ -224,7 +257,7 @@
 
 
 		flags = end ? O_CREAT|O_APPEND|O_WRONLY : O_CREAT|O_TRUNC|O_WRONLY;
-		s = ast_writefile( tmp, ext, NULL, flags , 0, 0644);
+		s = ast_writefile( realfilename, ext, NULL, flags , 0, 0644);
 
 
 		if (s) {