Rewrite t2p_process_jpeg_strip(), to fix CVE-2013-1960.

2 files changed, 70 insertions, 42 deletions

diff --git a/ChangeLog b/ChangeLog
index 54ce6e04..2d8bc7cd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2013-05-02  Tom Lane  <tgl@sss.pgh.pa.us>
 
+	* tools/tiff2pdf.c: Rewrite JPEG marker parsing in
+	t2p_process_jpeg_strip to be at least marginally competent.  The
+	approach is still fundamentally flawed, but at least now it won't
+	stomp all over memory when given bogus input.  Fixes CVE-2013-1960.
+
+2013-05-02  Tom Lane  <tgl@sss.pgh.pa.us>
+
 	* contrib/dbs/xtiff/xtiff.c, libtiff/tif_codec.c,
  	libtiff/tif_dirinfo.c, tools/rgb2ycbcr.c, tools/tiff2bw.c,
  	tools/tiff2pdf.c, tools/tiff2ps.c, tools/tiffcrop.c,
diff --git a/tools/tiff2pdf.c b/tools/tiff2pdf.c
index 966ddc33..c55b9270 100644
--- a/tools/tiff2pdf.c
+++ b/tools/tiff2pdf.c
@@ -1,4 +1,4 @@
-/* $Id: tiff2pdf.c,v 1.37.2.23 2013-05-02 14:44:44 tgl Exp $
+/* $Id: tiff2pdf.c,v 1.37.2.24 2013-05-02 14:54:18 tgl Exp $
  *
  * tiff2pdf - converts a TIFF image to a PDF document
  *
@@ -3337,33 +3337,56 @@ int t2p_process_jpeg_strip(
 	uint32 height){
 
 	tsize_t i=0;
-	uint16 ri =0;
-	uint16 v_samp=1;
-	uint16 h_samp=1;
-	int j=0;
-	
-	i++;
-	
-	while(i<(*striplength)){
+
+	while (i < *striplength) {
+		tsize_t datalen;
+		uint16 ri;
+		uint16 v_samp;
+		uint16 h_samp;
+		int j;
+		int ncomp;
+
+		/* marker header: one or more FFs */
+		if (strip[i] != 0xff)
+			return(0);
+		i++;
+		while (i < *striplength && strip[i] == 0xff)
+			i++;
+		if (i >= *striplength)
+			return(0);
+		/* SOI is the only pre-SOS marker without a length word */
+		if (strip[i] == 0xd8)
+			datalen = 0;
+		else {
+			if ((*striplength - i) <= 2)
+				return(0);
+			datalen = (strip[i+1] << 8) | strip[i+2];
+			if (datalen < 2 || datalen >= (*striplength - i))
+				return(0);
+		}
 		switch( strip[i] ){
-			case 0xd8:
-				/* SOI - start of image */
+			case 0xd8:	/* SOI - start of image */
 				_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), 2);
 				*bufferoffset+=2;
-				i+=2;
 				break;
-			case 0xc0:
-			case 0xc1:
-			case 0xc3:
-			case 0xc9:
-			case 0xca:
+			case 0xc0:	/* SOF0 */
+			case 0xc1:	/* SOF1 */
+			case 0xc3:	/* SOF3 */
+			case 0xc9:	/* SOF9 */
+			case 0xca:	/* SOF10 */
 				if(no==0){
-					_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), strip[i+2]+2);
-					for(j=0;j<buffer[*bufferoffset+9];j++){
-						if( (buffer[*bufferoffset+11+(2*j)]>>4) > h_samp) 
-							h_samp = (buffer[*bufferoffset+11+(2*j)]>>4);
-						if( (buffer[*bufferoffset+11+(2*j)] & 0x0f) > v_samp) 
-							v_samp = (buffer[*bufferoffset+11+(2*j)] & 0x0f);
+					_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), datalen+2);
+					ncomp = buffer[*bufferoffset+9];
+					if (ncomp < 1 || ncomp > 4)
+						return(0);
+					v_samp=1;
+					h_samp=1;
+					for(j=0;j<ncomp;j++){
+						uint16 samp = buffer[*bufferoffset+11+(3*j)];
+						if( (samp>>4) > h_samp) 
+							h_samp = (samp>>4);
+						if( (samp & 0x0f) > v_samp) 
+							v_samp = (samp & 0x0f);
 					}
 					v_samp*=8;
 					h_samp*=8;
@@ -3377,45 +3400,43 @@ int t2p_process_jpeg_strip(
                                           (unsigned char) ((height>>8) & 0xff);
 					buffer[*bufferoffset+6]=
                                             (unsigned char) (height & 0xff);
-					*bufferoffset+=strip[i+2]+2;
-					i+=strip[i+2]+2;
-
+					*bufferoffset+=datalen+2;
+					/* insert a DRI marker */
 					buffer[(*bufferoffset)++]=0xff;
 					buffer[(*bufferoffset)++]=0xdd;
 					buffer[(*bufferoffset)++]=0x00;
 					buffer[(*bufferoffset)++]=0x04;
 					buffer[(*bufferoffset)++]=(ri >> 8) & 0xff;
 					buffer[(*bufferoffset)++]= ri & 0xff;
-				} else {
-					i+=strip[i+2]+2;
 				}
 				break;
-			case 0xc4:
-			case 0xdb:
-				_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), strip[i+2]+2);
-				*bufferoffset+=strip[i+2]+2;
-				i+=strip[i+2]+2;
+			case 0xc4: /* DHT */
+			case 0xdb: /* DQT */
+				_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), datalen+2);
+				*bufferoffset+=datalen+2;
 				break;
-			case 0xda:
+			case 0xda: /* SOS */
 				if(no==0){
-					_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), strip[i+2]+2);
-					*bufferoffset+=strip[i+2]+2;
-					i+=strip[i+2]+2;
+					_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), datalen+2);
+					*bufferoffset+=datalen+2;
 				} else {
 					buffer[(*bufferoffset)++]=0xff;
 					buffer[(*bufferoffset)++]=
                                             (unsigned char)(0xd0 | ((no-1)%8));
-					i+=strip[i+2]+2;
 				}
-				_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i-1]), (*striplength)-i-1);
-				*bufferoffset+=(*striplength)-i-1;
+				i += datalen + 1;
+				/* copy remainder of strip */
+				_TIFFmemcpy(&(buffer[*bufferoffset]), &(strip[i]), *striplength - i);
+				*bufferoffset+= *striplength - i;
 				return(1);
 			default:
-				i+=strip[i+2]+2;
+				/* ignore any other marker */
+				break;
 		}
+		i += datalen + 1;
 	}
-	
 
+	/* failed to find SOS marker */
 	return(0);
 }
 #endif