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 ---
- To: debian-release@lists.debian.org
- Cc: team@security.debian.org, 309739@bugs.debian.org
- Subject: tiff security problem: planning to upload 3.7.2-3 to sid
- From: Jay Berkenbilt <qjb@debian.org>
- Date: Thu, 19 May 2005 06:08:37 -0400
- Message-id: <[🔎] 20050519060837.0548763463.qww314159@soup.in.ql.org>
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 NITEMSAttachment: pgpDxH0duFDGG.pgp
Description: PGP signature
--- End Message ---