Re: A security bug in Debian Squeeze libtiff (+ non-updated ia32-libs??)
On Sat, 7 Apr 2012, Mikulas Patocka wrote:
>
>
> On Sat, 7 Apr 2012, Mikulas Patocka wrote:
>
> > Hi
> >
> > There is a security bug in Debian Squeeze libtiff 3.9.4-5+sq.
> >
> > When loading corrupted images and with ElectricFence memory debugging
> > enabled, programs using libtiff crash.
> >
> > How to reproduce: Download corrupted images from here:
> > http://artax.karlin.mff.cuni.cz/~mikulas/debian-libtiff-bug/
> >
> > These tiff images were created by running fsfuzzer
> > (http://people.redhat.com/sgrubb/files/fsfuzzer-0.7.tar.gz) over normal
> > valid tiff images.
> >
> > Install electric-fence package from Debian.
> >
> > Run programs that use libtiff with electric fence, for example:
> >
> > LD_PRELOAD=/usr/lib/libefence.so links2 -g tiff1.tif
> >
> > LD_PRELOAD=/usr/lib/libefence.so xloadimage tiff1.tif
> >
> > LD_PRELOAD=/usr/lib/libefence.so xpaint tiff1.tif
> >
> > All the programs crash in TIFFReadDirectory (I tested it on amd64) --- so
> > it is a bug in libtiff.
> >
> >
> > I reproduced this bug on upstream libtiff 3.9.4, but couldn't reproduce it
> > on 3.9.5, 3.9.6 or 4.0.1 --- so the bug was fixed upstream and Debian
> > didn't backport it.
>
> After further fuzzing and testing with Electric Fence, I found out an
> image that crashes even upstream libtiff-3.9.6. So I'm sending the report
> to the upstream maintainers too.
>
> I placed the crashing image here:
>
> http://artax.karlin.mff.cuni.cz/~mikulas/debian-libtiff-bug/libtiff-3.9.6-crash.tif
>
> The crash happens here:
> #0 TIFFReadDirectory (tif=0x7f6f92434bc8) at tif_dirread.c:223
> 223 fip = tif->tif_fieldinfo[++fix];
>
> The apparent problem in the code:
>
> fip = tif->tif_fieldinfo[fix];
> while (dp->tdir_type != (unsigned short) fip->field_type
> && fix < tif->tif_nfields) {
> ^^^^^^^^^^ check that fix is smaller than tif->tif_nfields
> if (fip->field_type == TIFF_ANY) /*
> wildcard */
> break;
> fip = tif->tif_fieldinfo[++fix];
> ^^^^^^^^^^ increment fix by one and dereference tif->tif_fieldinfo[fix]
> !!! so we may be dereferencing one field after tif->tif_fieldinfo end
> if (fix >= tif->tif_nfields ||
> ^^^^^^^^^^ this check fix >= tif->tif_nfields comes too late, we already
> accessed the array beyond its end :-(
> fip->field_tag != dp->tdir_tag) {
> TIFFWarningExt(tif->tif_clientdata, module,
> "%s: wrong data type %d for \"%s\"; tag ignored",
> tif->tif_name, dp->tdir_type,
> tif->tif_fieldinfo[fix-1]->field_name);
> goto ignore;
> }
> }
>
> libtiff-4.0.1 doesn't crash and the above code seems to be rewritten
> there.
>
> Mikulas
And another buffer overflow, this time in libtiff-4.0.1, is here:
http://artax.karlin.mff.cuni.cz/~mikulas/debian-libtiff-bug/libtiff-4.0.1-crash.tif
To trigger the crash, you must set EF_ALIGNMENT=0 when using Electric
Fence (or it can be also triggered with llvm-3.1 address sanitizer).
(libtiff-3.9.6 doesn't crash, but the same buggy piece of code is there
too --- so the patch should be applied to both libtiff-4.0.1 and
libtiff-3.9.6).
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b5e445 in checkInkNamesString (tif=0x7fffef7d0b82, slen=1,
s=0x7fffef7defff "\001", '|' <repeats 16 times>,
"<<<||||<<<<<<<<<\034\034\034\030\030\030\030\030\030\030\030\030\020\030\030\030\020\020\020\030\030\020",
'\030' <repeats 13 times>, "\034\034", '|' <repeats 30 times>,
"<<<<||<<<<<<<<<<\034\034\030\030\030\030\030\b") at tif_dir.c:135
135 for (; *cp != '\0'; cp++)
(gdb) bt
#0 0x00007ffff7b5e445 in checkInkNamesString (tif=0x7fffef7d0b82, slen=1,
s=0x7fffef7defff "\001", '|' <repeats 16 times>,
"<<<||||<<<<<<<<<\034\034\034\030\030\030\030\030\030\030\030\030\020\030\030\030\020\020\020\030\030\020",
'\030' <repeats 13 times>, "\034\034", '|' <repeats 30 times>,
"<<<<||<<<<<<<<<<\034\034\030\030\030\030\030\b") at tif_dir.c:135
#1 0x00007ffff7b60229 in _TIFFVSetField (tif=0x7fffef7d0b82, tag=333,
ap=0x7fffffffce10) at tif_dir.c:409
#2 0x00007ffff7b6166b in TIFFVSetField (tif=0x7fffef7d0b82, tag=333,
ap=0x7fffffffce10) at tif_dir.c:790
#3 0x00007ffff7b61436 in TIFFSetField (tif=0x7fffef7d0b82, tag=333)
at tif_dir.c:734
#4 0x00007ffff7b6fd88 in TIFFFetchNormalTag (tif=0x7fffef7d0b82,
dp=0x7fffef7d6f10, recover=1) at tif_dirread.c:4972
#5 0x00007ffff7b6cd38 in TIFFReadDirectory (tif=0x7fffef7d0b82)
at tif_dirread.c:3808
#6 0x00007ffff7b9f1ff in TIFFClientOpen (
name=0x5214b8 "Prave si rek' svy posledni slova. A vybral sis k tomu
prihodny misto.", mode=0x5214af "r", clientdata=0x7fffef7bcf38,
readproc=0x4aae1d <tiff_read>, writeproc=0x4aaf02 <tiff_write>,
seekproc=0x4aaf41 <tiff_seek>, closeproc=0x4ab02c <tiff_close>,
sizeproc=0x4aadc0 <tiff_size>, mapproc=0x4ab0c0 <tiff_mmap>,
unmapproc=0x4ab140 <tiff_munmap>) at tif_open.c:466
The apparent problem in "checkInkNamesString" is that it first checks if
*cp != '\0' and then checks if cp is beyond buffer end (if cp >= ep).
This is a patch that reverses the checks: first check if it is beyond
buffer end, then check the value for zero.
Mikulas
---
libtiff/tif_dir.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Index: tiff-4.0.1/libtiff/tif_dir.c
===================================================================
--- tiff-4.0.1.orig/libtiff/tif_dir.c 2012-04-08 03:17:09.000000000 +0200
+++ tiff-4.0.1/libtiff/tif_dir.c 2012-04-08 03:19:19.000000000 +0200
@@ -132,9 +132,12 @@ checkInkNamesString(TIFF* tif, uint32 sl
const char* ep = s+slen;
const char* cp = s;
for (; i > 0; i--) {
- for (; *cp != '\0'; cp++)
+ while (1) {
if (cp >= ep)
goto bad;
+ if (!*cp) break;
+ cp++;
+ }
cp++; /* skip \0 */
}
return ((uint32)(cp-s));
Reply to: