Re: [SRM] Proposed NMU, gif2png to oldstable/stable (PRSC)

[Julien Cristau <jcristau@debian.org>]

On Fri, Feb 25, 2011 at 10:25:45 +0000, Jonathan Wiltshire wrote:

> --- gif2png-2.5.1.orig/debian/patches/10_fix_gif2png_c.dpatch
> +++ gif2png-2.5.1/debian/patches/10_fix_gif2png_c.dpatch
> @@ -0,0 +1,61 @@
> +#! /bin/sh /usr/share/dpatch/dpatch-run
> +## 10_fix_gif2png_c.dpatch by Erik Schanze <eriks@debian.org>
> +##
> +## All lines beginning with `## DP:' are a description of the patch.
> +## DP: - clarify if/else construct, because of compiler warning
> +## DP: - prevent buffer overflow with strcpy, closes: #550978
> +
> +@DPATCH@
> +diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' gif2png-2.5.4~/gif2png.c gif2png-2.5.4/gif2png.c
> +--- gif2png-2.5.4~/gif2png.c	2010-10-20 16:20:07.000000000 +0200
> ++++ gif2png-2.5.4/gif2png.c	2011-02-12 01:14:29.530909414 +0100
> +@@ -639,6 +639,7 @@
> +     int num_pics;
> +     struct GIFelement *start;
> +     int i, suppress_delete = FALSE;
> ++    int file_ext_max;
> +     char *file_ext;
> + 
> +     if (fp == NULL) return 1;
> +@@ -673,9 +675,11 @@
> + 
> +     /* create output filename */
> + 
> +-    strcpy(outname, fname);
> ++    strncpy( outname, fname, sizeof( outname ) );
> ++    outname[sizeof( outname ) - 1] = 0;
> + 
> +     file_ext = outname+strlen(outname)-4;
> ++    file_ext_max = sizeof(outname) - ( strlen(outname) - 4 );

Can file_ext_max ever be < 0?  If yes what's supposed to happen?

> +     if (strcmp(file_ext, ".gif") != 0 && strcmp(file_ext, ".GIF") != 0 &&
> + 	strcmp(file_ext, "_gif") != 0 && strcmp(file_ext, "_GIF") != 0) {
> + 	/* try to derive basename */
> +@@ -686,7 +690,8 @@
> + 	}
> + 	if (file_ext<outname || *file_ext != '.') {
> + 	    /* as a last resort, just add .png to the filename */
> +-	    file_ext = outname+strlen(outname);
> ++	    file_ext = outname + (size_t)(( strlen( outname ) <= sizeof( outname ) - 1 - 4 )? strlen( outname )
> ++										   : sizeof( outname ) - 1 - 4);
> + 	}
> +     }
> + 
> +@@ -708,7 +713,7 @@
> + 		fclose(fp);
> + 		++numpngs;
> + 		start = NULL;
> +-		sprintf(file_ext, ".p%02d", i);
> ++		snprintf(file_ext, file_ext_max - 1, ".p%02d", i);
> + 	    }
> + 	}
> +     }
> +@@ -863,7 +868,8 @@
> + 	}
> +     } else {
> + 	for (i = ac;i<argc; i++) {
> +-	    strcpy(name, argv[i]);
> ++	    strncpy(name, argv[i], sizeof name - sizeof ".gif");
> ++	    name[sizeof name - sizeof ".gif"] = '\0'; 
> + 	    if ((fp = fopen(name, "rb")) == NULL) {
> + 		/* retry with .gif appended */
> + 		strcat(name, ".gif");

Cheers,
Julien