Mon 19 Oct 2015 09:29:31 PM UTC, comment #9:
Thanks to both of you for your assistance! I've applied the patch to the git repository (with minor improvements).
|
Mon 19 Oct 2015 08:13:17 PM UTC, comment #8:
I've locally patched cid_load_glyph with
/* Adjustment for seed bytes. */
cs_offset = decoder->lenIV >= 0 ? (FT_UInt)decoder->lenIV : 0;
+ if (cs_offset > glyph_length)
+ {
+ FT_TRACE0(( "cid_load_glyph: invalid cs offsets\n" ));
+ error = FT_THROW( Invalid_Offset );
+ goto Exit;
+ }
/* Decrypt only if lenIV >= 0. */
Logically, this just checks that the cs_offset actually fits inside the glyph_length. In the example I was working with where the 'cs_offset' was 4 and 'glyph_length' was 2, it would have been necessary to somehow have read -2 bytes in order to satisfy the cs_offset.
|
Mon 19 Oct 2015 07:44:15 PM UTC, comment #7:
Oh, now I see that
glyph_length - cs_offset );
is accounting for the fact that the seed bytes are part of the glyph_length (since they're supposed to be part of the bytes for the glyph, they're not 'extra' bytes)
In that case it appears that there just needs to be a check that 'glyph_length >= cs_offset' before the call to parse_charstrings in cid_load_glyph.
|
Mon 19 Oct 2015 07:38:58 PM UTC, comment #6:
I've now also run into this. I've built with asan and set a breakpoint in gdb with 'break __asan_report_error', which allows one to break instead of exiting when asan reports an error. It appears that in t1_decoder_parse_charstrings 'charstring_len' is a very large number (seems to be a negative that is being treated as unsigned) resulting in 'limit' being bogus.
This appears to be the result of logic in cid_load_glyph. In the example I'm currenltly investigating, at the point where cid_load_glyph calls parse_charstrings, 'charstring' is 0x60200000eb10, and the value passed in as the second parameter is 0x60200000eb14, so 'cs_offset' is 4. However, 'glyph_length' here is only 2, so the third parameter is '-2' (as an unsigned).
Since the 'glyph_length' here is already supposed to be a length (not an offset) perhaps the line
glyph_length - cs_offset );
should just be
glyph_length );
On the other hand, this also seems to imply that the bounds checking above this would also need some attention to take into account the possible extra offset.
|
Mon 19 Oct 2015 05:35:13 PM UTC, comment #5:
ouch, sorry. Same bug, wrong reproducer.
Now running on crash-846ba5609a2246887b3fb26e8d40128b97d629b0
TTF driver
SFNT driver
not a font using the SFNT container format
Type 1 driver
not a Type 1 font
CFF driver
SFNT driver
not a font using the SFNT container format
not a CFF font header
CID driver
FT_Open_Face: New face object, adding to list
FT_Open_Face: Creating glyph slot
FT_Open_Face: Return 0
TTF driver
SFNT driver
not a font using the SFNT container format
Type 1 driver
not a Type 1 font
CFF driver
SFNT driver
not a font using the SFNT container format
not a CFF font header
CID driver
FT_Open_Face: New face object, adding to list
FT_Open_Face: Creating glyph slot
FT_Open_Face: Return 0
TTF driver
SFNT driver
not a font using the SFNT container format
Type 1 driver
not a Type 1 font
CFF driver
SFNT driver
not a font using the SFNT container format
not a CFF font header
CID driver
FT_Open_Face: New face object, adding to list
FT_Open_Face: Creating glyph slot
FT_New_GlyphSlot: Creating new slot object
FT_New_GlyphSlot: Return 0
FT_Open_Face: Creating size object
FT_Open_Face: Return 0
FT_Request_Metrics:
x scale: 4194 (0.063995)
y scale: 4194 (0.063995)
ascender: 1.000000
descender: -1.000000
height: 1.000000
max advance: 0.000000
x ppem: 1
y ppem: 1
FT_Request_Size (font driver's `request_size'):
x scale: 4194 (0.063995)
y scale: 4194 (0.063995)
ascender: 1.000000
descender: -1.000000
height: 1.000000
max advance: 0.000000
x ppem: 1
y ppem: 1
cid_load_glyph: glyph index 0
Start charstring
(0) 62 vlineto
(0)t1_decoder_parse_charstrings: invalid byte (27)
cid_load_glyph: glyph index 1
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 2
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 3
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 4
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 5
Start charstring
(0) 4294967282 90 4294967240 4294967221 4294967261 4294967240 4294967277 16 82 25t1_decoder_parse_charstrings: invalid byte (24)
cid_load_glyph: glyph index 6
cid_load_glyph: invalid glyph stream offsets
cid_load_glyph: glyph index 7
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 8
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 9
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 10
Start charstring
(0)==8316== Invalid read of size 1
==8316== at 0x46B5C6: t1_decoder_parse_charstrings (t1decode.c:455)
==8316== by 0x437288: cid_load_glyph (cidgload.c:165)
==8316== by 0x437694: cid_slot_load_glyph (cidgload.c:329)
==8316== by 0x40F114: FT_Load_Glyph (ftobjs.c:742)
|
Mon 19 Oct 2015 03:31:15 AM UTC, comment #4:
Thanks, but is this really a trace of file #35208 (`crash-846ba5609a2246887b3fb26e8d40128b97d629b0') using the current version of the ftfuzzer program, together with a current version of the library? I get completely different values and tracing messages:
+verbose+
...
cid_load_glyph: glyph index 0
Start charstring
(0) 62 vlineto
(0)t1_decoder_parse_charstrings: invalid byte (27)
cid_load_glyph: glyph index 1
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 2
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 3
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 4
x advance: 0
y advance: 0
linear x advance: 0
linear y advance: 66514
cid_load_glyph: glyph index 5
Start charstring
(0) -14 90 -56 -75 -35 -56 -19 16 82 25t1_decoder_parse_charstrings: invalid byte (24)
cid_load_glyph: glyph index 6
cid_load_glyph: invalid glyph stream offsets
cid_load_glyph: glyph index 7
...
-verbose-
Just to be sure, I've just checked the SHA1 value of that file, and it is indeed indentical to the file's name.
|
Sun 18 Oct 2015 12:05:26 AM UTC, comment #3:
(running debug build under valgrind)
TTF driver
SFNT driver
not a font using the SFNT container format
Type 1 driver
not a Type 1 font
CFF driver
SFNT driver
not a font using the SFNT container format
not a CFF font header
CID driver
FT_Open_Face: New face object, adding to list
FT_Open_Face: Creating glyph slot
FT_Open_Face: Return 0
TTF driver
SFNT driver
not a font using the SFNT container format
Type 1 driver
not a Type 1 font
CFF driver
SFNT driver
not a font using the SFNT container format
not a CFF font header
CID driver
FT_Open_Face: New face object, adding to list
FT_Open_Face: Creating glyph slot
FT_Open_Face: Return 0
TTF driver
SFNT driver
not a font using the SFNT container format
Type 1 driver
not a Type 1 font
CFF driver
SFNT driver
not a font using the SFNT container format
not a CFF font header
CID driver
FT_Open_Face: New face object, adding to list
FT_Open_Face: Creating glyph slot
FT_New_GlyphSlot: Creating new slot object
FT_New_GlyphSlot: Return 0
FT_Open_Face: Creating size object
FT_Open_Face: Return 0
FT_Request_Metrics:
x scale: 4194 (0.063995)
y scale: 4194 (0.063995)
ascender: 1.000000
descender: -1.000000
height: 1.000000
max advance: 0.000000
x ppem: 1
y ppem: 1
FT_Request_Size (font driver's `request_size'):
x scale: 4194 (0.063995)
y scale: 4194 (0.063995)
ascender: 1.000000
descender: -1.000000
height: 1.000000
max advance: 0.000000
x ppem: 1
y ppem: 1
cid_load_glyph: glyph index 0
Start charstring
(0) 4294967200 4294967289 69 22306593 4294967213 4294967218 56 4294967282 96t1_decoder_parse_charstrings: no `div' after large integer
2924727959 55 4294967295t1_decoder_parse_charstrings: invalid escape (12+248)
cid_load_glyph: glyph index 1
FT_Stream_ReadAt: invalid read; expected 11 bytes, got 2
cid_load_glyph: glyph index 2
cid_load_glyph: invalid glyph stream offsets
cid_load_glyph: glyph index 3
cid_load_glyph: invalid glyph stream offsets
cid_load_glyph: glyph index 4
Start charstring
(0) 82 52 78t1_decoder_parse_charstrings: invalid byte (0)
cid_load_glyph: glyph index 5
cid_load_glyph: invalid glyph stream offsets
cid_load_glyph: glyph index 6
Start charstring
(0)==21717== Invalid read of size 1
==21717== at 0x46B5C6: t1_decoder_parse_charstrings (t1decode.c:455)
==21717== by 0x437288: cid_load_glyph (cidgload.c:165)
==21717== by 0x437694: cid_slot_load_glyph (cidgload.c:329)
==21717== by 0x40F114: FT_Load_Glyph (ftobjs.c:742)
==21717== by 0x408119: LLVMFuzzerTestOneInput (in /usr/local/google/home/kcc/fuzz/freetype/freetype2_repro_orig)
==21717== by 0x409240: main (in /usr/local/google/home/kcc/fuzz/freetype/freetype2_repro_orig)
==21717== Address 0x6eed144 is 2 bytes after a block of size 2 alloc'd
==21717== at 0x4C2ABBD: malloc (vg_replace_malloc.c:296)
==21717== by 0x411614: ft_mem_qalloc (ftutil.c:76)
==21717== by 0x411697: ft_mem_alloc (ftutil.c:55)
==21717== by 0x4374F0: cid_load_glyph (cidgload.c:130)
==21717== by 0x437694: cid_slot_load_glyph (cidgload.c:329)
==21717== by 0x40F114: FT_Load_Glyph (ftobjs.c:742)
|
Sat 17 Oct 2015 06:23:34 AM UTC, comment #2:
The problem with line 522 is now fixed in git, thanks.
On my 32bit GNU/Linux box, however, I couldn't reproduce the problem in line 455, neither with clang nor with gcc 5.2.
Please recompile the library with -DFT_DEBUG_LEVEL_ERROR -DFT_DEBUG_LEVEL_TRACE, run
and attach the output file. This helps me identify the exact spot of the overflow.
|
Fri 16 Oct 2015 05:46:14 AM UTC, comment #1:
Maybe a variation of this one, maybe something else;
==31864==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200018a015 at pc 0x0000007ae73f bp 0x7ffc9bfcded0 sp 0x7ffc9bfcdec8
READ of size 1 at 0x60200018a015 thread T0
#0 0x7ae73e in t1_decoder_parse_charstrings /src/psaux/t1decode.c:522:18
#1 0x6367e2 in cid_load_glyph /src/cid/cidgload.c:160:15
#2 0x63269b in cid_slot_load_glyph /src/cid/cidgload.c:324:13
#3 0x4ffb58 in FT_Load_Glyph /src/base/ftobjs.c:742:15
#4 0x4d962f in LLVMFuzzerTestOneInput (_fuzz+0x4d962f)
#5 0x4e00b8 in fuzzer::SimpleUserSuppliedFuzzer::TargetFunction(unsigned char const*, unsigned long) (_fuzz+0x4e00b8)
#6 0x4edf31 in fuzzer::Fuzzer::ExecuteCallback(std::vector<unsigned char, std::allocator<unsigned char> > const&) (_fuzz+0x4edf31)
#7 0x4edbe8 in fuzzer::Fuzzer::RunOneMaximizeTotalCoverage(std::vector<unsigned char, std::allocator<unsigned char> > const&) (_fuzz+0x4edbe8)
#8 0x4ed348 in fuzzer::Fuzzer::RunOne(std::vector<unsigned char, std::allocator<unsigned char> > const&) (_fuzz+0x4ed348)
#9 0x4edd07 in fuzzer::Fuzzer::RunOneAndUpdateCorpus(std::vector<unsigned char, std::allocator<unsigned char> >&) (_fuzz+0x4edd07)
#10 0x4ee4a6 in fuzzer::Fuzzer::MutateAndTestOne(std::vector<unsigned char, std::allocator<unsigned char> >*) (_fuzz+0x4ee4a6)
#11 0x4eea64 in fuzzer::Fuzzer::Loop() (_fuzz+0x4eea64)
#12 0x4de268 in fuzzer::FuzzerDriver(std::vector<std::string, std::allocator<std::string> > const&, fuzzer::UserSuppliedFuzzer&) (_fuzz+0x4de268)
#13 0x4dd7f6 in fuzzer::FuzzerDriver(int, char**, fuzzer::UserSuppliedFuzzer&) (_fuzz+0x4dd7f6)
#14 0x4dd72d in fuzzer::FuzzerDriver(int, char*, int ()(unsigned char const*, unsigned long)) (_fuzz+0x4dd72d)
#15 0x4f6dcb in main (_fuzz+0x4f6dcb)
#16 0x7fb693575ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#17 0x41eafe in _start (_fuzz+0x41eafe)
0x60200018a015 is located 0 bytes to the right of 5-byte region [0x60200018a010,0x60200018a015)
allocated by thread T0 here:
#0 0x4ae79b in __interceptor_malloc /usr/local/google/home/kcc/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
#1 0x505d1e in ft_mem_qalloc /src/base/ftutil.c:76:15
#2 0x505d1e in ft_mem_alloc /src/base/ftutil.c:55
#3 0x63269b in cid_slot_load_glyph /src/cid/cidgload.c:324:13
#4 0x4ffb58 in FT_Load_Glyph /src/base/ftobjs.c:742:15
(file #35209)
|
Fri 16 Oct 2015 05:43:09 AM UTC, original submission:
found by libFuzzer+AddressSanitizer, repro attached.
==417==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000eef2 at pc 0x00000078baef bp 0x7ffc23fc2d10 sp 0x7ffc23fc2d08
READ of size 1 at 0x60200000eef2 thread T0
#0 0x78baee in t1_decoder_parse_charstrings src/psaux/t1decode.c:455:16
#1 0x613ab2 in cid_load_glyph src/cid/cidgload.c:160:15
#2 0x60f96b in cid_slot_load_glyph src/cid/cidgload.c:324:13
#3 0x4dce28 in FT_Load_Glyph src/base/ftobjs.c:742:15
#4 0x4d75ef in LLVMFuzzerTestOneInput (/usr/local/google/home/kcc/fuzz/freetype/freetype2_repro+0x4d75ef)
#5 0x4db0ab in main (/usr/local/google/home/kcc/fuzz/freetype/freetype2_repro+0x4db0ab)
#6 0x7fc6043faec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#7 0x41cabe in _start (/usr/local/google/home/kcc/fuzz/freetype/freetype2_repro+0x41cabe)
0x60200000eef2 is located 1 bytes to the right of 1-byte region [0x60200000eef0,0x60200000eef1)
allocated by thread T0 here:
#0 0x4ac75b in __interceptor_malloc /usr/local/google/home/kcc/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
#1 0x4e2fee in ft_mem_qalloc src/base/ftutil.c:76:15
#2 0x4e2fee in ft_mem_alloc src/base/ftutil.c:55
#3 0x60f96b in cid_slot_load_glyph src/cid/cidgload.c:324:13
#4 0x4dce28 in FT_Load_Glyph src/base/ftobjs.c:742:15
|