[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

please approve tiff-3.7.2-3 (fix to CAN-2005-1544)



Please approve tiff-3.7.2-3 for sarge which fixes a security bug (RC
bug 309739, CAN-2005-1544).  Details including patch and analysis
below.  Executive summary: libtiff failed to validate input based on
samples per pixel; retrieved fix from upstream, applied, and tested.
I've attached two patches: one with -w so that it's easier to read for
analysis, and the real one that has insignificant changes in
surrounding text because of indentation.  I will be leaving for my
vacation in a few hours and most likely won't have email access again
for several days.  Thanks.

-- 
Jay Berkenbilt <qjb@debian.org>


--- Begin Message ---
Unless I hear otherwise, I'm planning on uploading tiff 3.7.2-3
(currently prepared and ready; closes RC security bug 309739,
urgency=high) to sid this morning and to then request its approval for
sarge.  If you would like me to handle this in some other way, please
let me know ASAP.  I'm leaving for vacation today and intend to do
this upload in the next few hours.

I'm attaching two versions of the patch.  The first one,
CAN-2005-1544-readable.patch, was generated with diff -uw.  It ignores
whitespace, so it would not apply properly to the source, but it is
compact and easy to read for the benefit of the release team.  The
second patch is the real patch applied to the code.  It is larger than
the change warrants because of indentation changes in the code, but is
functionally identical.

Both versions of the patch were generated from upstream's CVS after
verifying with upstream's bug tracking system and looking at the patch
applied to the gentoo packages.  The change is quite straightforward
and is pretty self-evident from reading the patch generated without
extraneous whitespace changes.  When reading an array whose length is
supposed to be the value of bits per sample, the old code failed to
validate the actual amount of data in the file.  The new code
allocates the amount of memory required for what's actually in the
file and ensures that the samples value doesn't exceed that amount.
The same code change is repeated three times: once for short, once for
long, and once for "any" in tif_dirread.c.  I'm hoping that, with this
description, you'll be able to approve the change at a glance.  I've
also done a quick check over tif_dirread.c to make sure that these are
really the only occurrences of the offending code and that the three
functions really do need exactly the same fix and have also done my
usual testing of the libraries and tools to test for unintended
consequences.  This patch are the only difference between 3.7.2-2 and
3.7.2-3.  (It is quite clear anyway that there is no possibility of
any ABI breakage.)

Thanks for your consideration.

-- 
Jay Berkenbilt <qjb@debian.org>

Index: libtiff/tif_dirread.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_dirread.c,v
retrieving revision 1.51
retrieving revision 1.53
diff -w -u -r1.51 -r1.53
--- libtiff/tif_dirread.c	3 Mar 2005 16:00:01 -0000	1.51
+++ libtiff/tif_dirread.c	6 May 2005 14:35:50 -0000	1.53
@@ -1310,12 +1310,16 @@
 		uint16 buf[10];
 		uint16* v = buf;
 
-		if (samples > NITEMS(buf))
-			v = (uint16*) CheckMalloc(tif, samples, sizeof(uint16),
+        if (dir->tdir_count > NITEMS(buf))
+            v = (uint16*) CheckMalloc(tif, dir->tdir_count, sizeof(uint16),
 						  "to fetch per-sample values");
 		if (v && TIFFFetchShortArray(tif, dir, v)) {
 			uint16 i;
-			for (i = 1; i < samples; i++)
+            int check_count = dir->tdir_count;
+            if( samples < check_count )
+                check_count = samples;
+
+            for (i = 1; i < check_count; i++)
 				if (v[i] != v[0]) {
 					TIFFError(tif->tif_name,
 		"Cannot handle different per-sample values for field \"%s\"",
@@ -1347,12 +1351,16 @@
 		uint32 buf[10];
 		uint32* v = buf;
 
-		if (samples > NITEMS(buf))
-			v = (uint32*) CheckMalloc(tif, samples, sizeof(uint32),
+        if (dir->tdir_count > NITEMS(buf))
+            v = (uint32*) CheckMalloc(tif, dir->tdir_count, sizeof(uint32),
 						  "to fetch per-sample values");
 		if (v && TIFFFetchLongArray(tif, dir, v)) {
 			uint16 i;
-			for (i = 1; i < samples; i++)
+            int check_count = dir->tdir_count;
+
+            if( samples < check_count )
+                check_count = samples;
+            for (i = 1; i < check_count; i++)
 				if (v[i] != v[0]) {
 					TIFFError(tif->tif_name,
 		"Cannot handle different per-sample values for field \"%s\"",
@@ -1384,12 +1392,16 @@
 		double buf[10];
 		double* v = buf;
 
-		if (samples > NITEMS(buf))
-			v = (double*) CheckMalloc(tif, samples, sizeof (double),
+        if (dir->tdir_count > NITEMS(buf))
+            v = (double*) CheckMalloc(tif, dir->tdir_count, sizeof (double),
 						  "to fetch per-sample values");
 		if (v && TIFFFetchAnyArray(tif, dir, v)) {
 			uint16 i;
-			for (i = 1; i < samples; i++)
+            int check_count = dir->tdir_count;
+            if( samples < check_count )
+                check_count = samples;
+
+            for (i = 1; i < check_count; i++)
 				if (v[i] != v[0]) {
 					TIFFError(tif->tif_name,
 		"Cannot handle different per-sample values for field \"%s\"",
Index: libtiff/tif_dirread.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_dirread.c,v
retrieving revision 1.51
retrieving revision 1.53
diff -u -r1.51 -r1.53
--- libtiff/tif_dirread.c	3 Mar 2005 16:00:01 -0000	1.51
+++ libtiff/tif_dirread.c	6 May 2005 14:35:50 -0000	1.53
@@ -1303,33 +1303,37 @@
 static int
 TIFFFetchPerSampleShorts(TIFF* tif, TIFFDirEntry* dir, uint16* pl)
 {
-	uint16 samples = tif->tif_dir.td_samplesperpixel;
-	int status = 0;
+    uint16 samples = tif->tif_dir.td_samplesperpixel;
+    int status = 0;
 
-	if (CheckDirCount(tif, dir, (uint32) samples)) {
-		uint16 buf[10];
-		uint16* v = buf;
-
-		if (samples > NITEMS(buf))
-			v = (uint16*) CheckMalloc(tif, samples, sizeof(uint16),
-						  "to fetch per-sample values");
-		if (v && TIFFFetchShortArray(tif, dir, v)) {
-			uint16 i;
-			for (i = 1; i < samples; i++)
-				if (v[i] != v[0]) {
-					TIFFError(tif->tif_name,
-		"Cannot handle different per-sample values for field \"%s\"",
-			   _TIFFFieldWithTag(tif, dir->tdir_tag)->field_name);
-					goto bad;
-				}
-			*pl = v[0];
-			status = 1;
-		}
-	bad:
-		if (v && v != buf)
-			_TIFFfree(v);
-	}
-	return (status);
+    if (CheckDirCount(tif, dir, (uint32) samples)) {
+        uint16 buf[10];
+        uint16* v = buf;
+
+        if (dir->tdir_count > NITEMS(buf))
+            v = (uint16*) CheckMalloc(tif, dir->tdir_count, sizeof(uint16),
+                                      "to fetch per-sample values");
+        if (v && TIFFFetchShortArray(tif, dir, v)) {
+            uint16 i;
+            int check_count = dir->tdir_count;
+            if( samples < check_count )
+                check_count = samples;
+
+            for (i = 1; i < check_count; i++)
+                if (v[i] != v[0]) {
+                    TIFFError(tif->tif_name,
+                              "Cannot handle different per-sample values for field \"%s\"",
+                              _TIFFFieldWithTag(tif, dir->tdir_tag)->field_name);
+                    goto bad;
+                }
+            *pl = v[0];
+            status = 1;
+        }
+      bad:
+        if (v && v != buf)
+            _TIFFfree(v);
+    }
+    return (status);
 }
 
 /*
@@ -1340,33 +1344,37 @@
 static int
 TIFFFetchPerSampleLongs(TIFF* tif, TIFFDirEntry* dir, uint32* pl)
 {
-	uint16 samples = tif->tif_dir.td_samplesperpixel;
-	int status = 0;
+    uint16 samples = tif->tif_dir.td_samplesperpixel;
+    int status = 0;
 
-	if (CheckDirCount(tif, dir, (uint32) samples)) {
-		uint32 buf[10];
-		uint32* v = buf;
-
-		if (samples > NITEMS(buf))
-			v = (uint32*) CheckMalloc(tif, samples, sizeof(uint32),
-						  "to fetch per-sample values");
-		if (v && TIFFFetchLongArray(tif, dir, v)) {
-			uint16 i;
-			for (i = 1; i < samples; i++)
-				if (v[i] != v[0]) {
-					TIFFError(tif->tif_name,
-		"Cannot handle different per-sample values for field \"%s\"",
-			   _TIFFFieldWithTag(tif, dir->tdir_tag)->field_name);
-					goto bad;
-				}
-			*pl = v[0];
-			status = 1;
-		}
-	bad:
-		if (v && v != buf)
-			_TIFFfree(v);
-	}
-	return (status);
+    if (CheckDirCount(tif, dir, (uint32) samples)) {
+        uint32 buf[10];
+        uint32* v = buf;
+
+        if (dir->tdir_count > NITEMS(buf))
+            v = (uint32*) CheckMalloc(tif, dir->tdir_count, sizeof(uint32),
+                                      "to fetch per-sample values");
+        if (v && TIFFFetchLongArray(tif, dir, v)) {
+            uint16 i;
+            int check_count = dir->tdir_count;
+
+            if( samples < check_count )
+                check_count = samples;
+            for (i = 1; i < check_count; i++)
+                if (v[i] != v[0]) {
+                    TIFFError(tif->tif_name,
+                              "Cannot handle different per-sample values for field \"%s\"",
+                              _TIFFFieldWithTag(tif, dir->tdir_tag)->field_name);
+                    goto bad;
+                }
+            *pl = v[0];
+            status = 1;
+        }
+      bad:
+        if (v && v != buf)
+            _TIFFfree(v);
+    }
+    return (status);
 }
 
 /*
@@ -1377,33 +1385,37 @@
 static int
 TIFFFetchPerSampleAnys(TIFF* tif, TIFFDirEntry* dir, double* pl)
 {
-	uint16 samples = tif->tif_dir.td_samplesperpixel;
-	int status = 0;
+    uint16 samples = tif->tif_dir.td_samplesperpixel;
+    int status = 0;
 
-	if (CheckDirCount(tif, dir, (uint32) samples)) {
-		double buf[10];
-		double* v = buf;
-
-		if (samples > NITEMS(buf))
-			v = (double*) CheckMalloc(tif, samples, sizeof (double),
-						  "to fetch per-sample values");
-		if (v && TIFFFetchAnyArray(tif, dir, v)) {
-			uint16 i;
-			for (i = 1; i < samples; i++)
-				if (v[i] != v[0]) {
-					TIFFError(tif->tif_name,
-		"Cannot handle different per-sample values for field \"%s\"",
-			   _TIFFFieldWithTag(tif, dir->tdir_tag)->field_name);
-					goto bad;
-				}
-			*pl = v[0];
-			status = 1;
-		}
-	bad:
-		if (v && v != buf)
-			_TIFFfree(v);
-	}
-	return (status);
+    if (CheckDirCount(tif, dir, (uint32) samples)) {
+        double buf[10];
+        double* v = buf;
+
+        if (dir->tdir_count > NITEMS(buf))
+            v = (double*) CheckMalloc(tif, dir->tdir_count, sizeof (double),
+                                      "to fetch per-sample values");
+        if (v && TIFFFetchAnyArray(tif, dir, v)) {
+            uint16 i;
+            int check_count = dir->tdir_count;
+            if( samples < check_count )
+                check_count = samples;
+
+            for (i = 1; i < check_count; i++)
+                if (v[i] != v[0]) {
+                    TIFFError(tif->tif_name,
+                              "Cannot handle different per-sample values for field \"%s\"",
+                              _TIFFFieldWithTag(tif, dir->tdir_tag)->field_name);
+                    goto bad;
+                }
+            *pl = v[0];
+            status = 1;
+        }
+      bad:
+        if (v && v != buf)
+            _TIFFfree(v);
+    }
+    return (status);
 }
 #undef NITEMS
 

Attachment: pgp71lq4eJAxH.pgp
Description: PGP signature


--- End Message ---

Reply to: