Re: Bug#349653: xmame: exploitable buffer overflows [CVE-2006-0176]

[Pierre Riteau <pierre.riteau@free.fr>]

On Wed, Apr 12, 2006 at 05:59:45PM +0200, Moritz Muehlenhoff wrote:
> > The Debian security FAQ says that non-free is not supported, and I
> > understand why. But it also says that if it is fixable, an update can be
> > made. There were (a few) non-free security updates in the past.
> > 
> > I see that Bruno is alive :) If he reviews my patch for Sarge and if the
> > security buildds have CPU time available, is it possible to release an
> > update? I can write a part of the DSA if you want.
> 
> We're all quite busy with updates for free packages. I'd recommend
> instead to include the update in the r2 Sarge stable update scheduled
> for next week. You can contact the stable release managers through
> debian-release@lists.debian.org.

Here is the point: there is a security flaw in xmame package
(non-free), which could lead to a privilege escalation (some xmame-*
packages are installed setuid root). As Moritz said they are already
quite busy with free packages, so I would like to know if it is possible
to include the fix in 3.1r2 (maybe it is a little late now?).

Attached is my patch to Sarge version of xmame. Of course I would like
the maintainer to give his opinion about it. Bruno?

Regards,
Pierre Riteau

--- xmame-0.86.orig/src/unix/dirio.c
+++ xmame-0.86/src/unix/dirio.c
@@ -184,7 +184,7 @@
 #else
 	getcwd(cwd, MAXPATHL);
 #endif
-	strcat(cwd, "/");
+	strncat(cwd, "/", sizeof(cwd) - strlen(cwd) - 1);
 	return cwd;
 }
 
--- xmame-0.86.orig/src/unix/fileio.c
+++ xmame-0.86/src/unix/fileio.c
@@ -412,7 +412,7 @@
 /*	compose_path */
 /*============================================================ */
 
-static void compose_path(char *output, int pathtype, int pathindex, const char *filename)
+static void compose_path(char *output, size_t outputlen, int pathtype, int pathindex, const char *filename)
 {
 	const char *basepath = get_path_for_filetype(pathtype, pathindex, NULL);
 	char *p;
@@ -425,10 +425,10 @@
 	/* compose the full path */
 	*output = 0;
 	if (basepath)
-		strcat(output, basepath);
+		strncat(output, basepath, outputlen - strlen(output) - 1);
 	if (*output && !is_pathsep(output[strlen(output) - 1]))
-		strcat(output, "/");
-	strcat(output, filename);
+		strncat(output, "/", outputlen - strlen(output) - 1);
+	strncat(output, filename, outputlen - strlen(output) - 1);
 
 	/* convert backslashes to forward slashes */
 	for (p = output; *p; p++)
@@ -463,7 +463,7 @@
 	char fullpath[1024];
 
 	/* compose the full path */
-	compose_path(fullpath, pathtype, pathindex, filename);
+	compose_path(fullpath, sizeof(fullpath), pathtype, pathindex, filename);
 
 	/* get the file attributes */
 	if (stat(fullpath, &buf))
@@ -499,7 +499,7 @@
 	memset(file, 0, sizeof(*file));
 
 	/* compose the full path */
-	compose_path(fullpath, pathtype, pathindex, filename);
+	compose_path(fullpath, sizeof(fullpath), pathtype, pathindex, filename);
 
 	/* attempt to open the file */
 	file->fileptr = fopen(fullpath, mode);
@@ -699,7 +699,7 @@
 	char fullpath[1024];
 
 	/* compose the full path */
-	compose_path(fullpath, pathtype, pathindex, dirname);
+	compose_path(fullpath, sizeof(fullpath), pathtype, pathindex, dirname);
 
 	return check_and_create_dir(fullpath) ? 0 : 1;
 }
--- xmame-0.86.orig/src/unix/joystick-drivers/joy_i386.c
+++ xmame-0.86/src/unix/joystick-drivers/joy_i386.c
@@ -83,7 +83,7 @@
 	fprintf (stderr_file, "I386 joystick interface initialization...\n");
 	for (i = first_dev; i <= last_dev; i++)
 	{
-		sprintf (devname, "%s%d", joy_dev, i);
+		snprintf (devname, sizeof(devname), "%s%d", joy_dev, i);
 		if ((joy_data[i].fd = open (devname, O_RDONLY)) >= 0)
 		{
 			if(joytype != JOY_I386NEW)
--- xmame-0.86.orig/src/unix/config.c
+++ xmame-0.86/src/unix/config.c
@@ -627,7 +627,7 @@
 		INP_HEADER inp_header;
 
 		memset(&inp_header, '\0', sizeof(INP_HEADER));
-		strcpy(inp_header.name, drivers[game_index]->name);
+		strncpy(inp_header.name, drivers[game_index]->name, sizeof(inp_header.name) - 1);
 		mame_fwrite(options.record, &inp_header, sizeof(INP_HEADER));
 	}
 
--- xmame-0.86.orig/src/fileio.c
+++ xmame-0.86/src/fileio.c
@@ -325,16 +325,20 @@
 	int pathindex;
 
 	/* copy the filename and add an extension */
-	strcpy(modified_filename, filename);
+	strncpy(modified_filename, filename, sizeof(modified_filename) - 1);
+	modified_filename[sizeof(modified_filename) - 1] = 0;
 	if (extension)
 	{
 		char *p = strchr(modified_filename, '.');
 		if (p)
-			strcpy(p, extension);
+		{
+			strncpy(p, extension, sizeof(modified_filename) - (p - modified_filename) - 1);
+			modified_filename[sizeof(modified_filename) - 1] = 0;
+		}
 		else
 		{
-			strcat(modified_filename, ".");
-			strcat(modified_filename, extension);
+			strncat(modified_filename, ".", sizeof(modified_filename) - strlen(modified_filename) - 1);
+			strncat(modified_filename, extension, sizeof(modified_filename) - strlen(modified_filename) - 1);
 		}
 	}
 
@@ -344,19 +348,19 @@
 		char name[256];
 
 		/* first check the raw filename, in case we're looking for a directory */
-		sprintf(name, "%s", filename);
+		snprintf(name, sizeof(name), "%s", filename);
 		LOG(("mame_faccess: trying %s\n", name));
 		if (osd_get_path_info(filetype, pathindex, name) != PATH_NOT_FOUND)
 			return 1;
 
 		/* try again with a .zip extension */
-		sprintf(name, "%s.zip", filename);
+		snprintf(name, sizeof(name), "%s.zip", filename);
 		LOG(("mame_faccess: trying %s\n", name));
 		if (osd_get_path_info(filetype, pathindex, name) != PATH_NOT_FOUND)
 			return 1;
 
 		/* does such a directory (or file) exist? */
-		sprintf(name, "%s", modified_filename);
+		snprintf(name, sizeof(name), "%s", modified_filename);
 		LOG(("mame_faccess: trying %s\n", name));
 		if (osd_get_path_info(filetype, pathindex, name) != PATH_NOT_FOUND)
 			return 1;
@@ -743,7 +747,7 @@
 	compose_path
 ***************************************************************************/
 
-INLINE void compose_path(char *output, const char *gamename, const char *filename, const char *extension)
+INLINE void compose_path(char *output, size_t outputlen, const char *gamename, const char *filename, const char *extension)
 {
 	char *filename_base = output;
 	*output = 0;
@@ -751,7 +755,8 @@
 #ifdef MESS
 	if (filename && osd_is_absolute_path(filename))
 	{
-		strcpy(output, filename);
+		strncpy(output, filename, outputlen - 1);
+		output[outputlen - 1] = 0;
 		return;
 	}
 #endif
@@ -759,23 +764,23 @@
 	/* if there's a gamename, add that; only add a '/' if there is a filename as well */
 	if (gamename)
 	{
-		strcat(output, gamename);
+		strncat(output, gamename, outputlen - strlen(output) - 1);
 		if (filename)
 		{
-			strcat(output, "/");
+			strncat(output, "/", outputlen - strlen(output) - 1);
 			filename_base = &output[strlen(output)];
 		}
 	}
 
 	/* if there's a filename, add that */
 	if (filename)
-		strcat(output, filename);
+		strncat(output, filename, outputlen - strlen(output) - 1);
 
 	/* if there's no extension in the filename, add the extension */
 	if (extension && !strchr(filename_base, '.'))
 	{
-		strcat(output, ".");
-		strcat(output, extension);
+		strncat(output, ".", outputlen - strlen(output) - 1);
+		strncat(output, extension, outputlen - strlen(output) - 1);
 	}
 }
 
@@ -920,7 +925,7 @@
 		/* ----------------- STEP 1: OPEN THE FILE RAW -------------------- */
 
 		/* first look for path/gamename as a directory */
-		compose_path(name, gamename, NULL, NULL);
+		compose_path(name, sizeof(name), gamename, NULL, NULL);
 		LOG(("Trying %s\n", name));
 
 #ifdef MESS
@@ -939,7 +944,7 @@
 		if (*name == 0 || osd_get_path_info(pathtype, pathindex, name) == PATH_IS_DIRECTORY)
 		{
 			/* now look for path/gamename/filename.ext */
-			compose_path(name, gamename, filename, extension);
+			compose_path(name, sizeof(name), gamename, filename, extension);
 
 			/* if we need checksums, load it into RAM and compute it along the way */
 			if (flags & FILEFLAG_HASH)
@@ -1026,7 +1031,7 @@
 		if (!(flags & (FILEFLAG_OPENWRITE | FILEFLAG_NOZIP)))
 		{
 			/* first look for path/gamename.zip */
-			compose_path(name, gamename, NULL, "zip");
+			compose_path(name, sizeof(name), gamename, NULL, "zip");
 			LOG(("Trying %s file\n", name));
 
 			/* if the ZIP file exists, proceed */
@@ -1035,7 +1040,7 @@
 				UINT32 ziplength;
 
 				/* if the file was able to be extracted from the ZIP, continue */
-				compose_path(tempname, NULL, filename, extension);
+				compose_path(tempname, sizeof(tempname), NULL, filename, extension);
 
 				/* verify-only case */
 				if (flags & FILEFLAG_VERIFY_ONLY)