1301250 - libjpeg-turbo: index out of bounds in [@start_pass_huff_decoder]

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.jpg

I found this while fuzzing a 32-bit build of libjpeg-turbo revision 8ce2c9119a995ef6280f8bba375aac7effb9b571. 

This was found with UBSan with build flags:
CFLAGS="-fsanitize=undefined -fno-sanitize-recover=undefined -m32 -g -O2" LDFLAGS="-fsanitize=undefined -m32" CC=clang

../jdhuff.c:114:38: runtime error: index 14 out of bounds for type 'd_derived_tbl *[4]'
    #0 0x82cc6de in start_pass_huff_decoder /home/user/code/libjpeg-turbo/build/../jdhuff.c:114:38
    #1 0x818fc3e in start_input_pass /home/user/code/libjpeg-turbo/build/../jdinput.c:282:3
    #2 0x818ad10 in consume_markers /home/user/code/libjpeg-turbo/build/../jdinput.c:334:7
    #3 0x81736f4 in jpeg_start_decompress /home/user/code/libjpeg-turbo/build/../jdapistd.c:65:19
    #4 0x8136788 in fuzz /home/user/code/libjpeg-turbo/build/../djpeg.c:179:10
    #5 0x8137630 in main /home/user/code/libjpeg-turbo/build/../djpeg.c:218:3
    #6 0xf754f636 in __libc_start_main /build/glibc-az1lHK/glibc-2.23/csu/../csu/libc-start.c:291
    #7 0x8061d57 in _start (/home/user/workspace/libjpeg/djpeg+0x8061d57)

Comment 1

[Daniel Veditz [:dveditz]]

What happens in a Firefox ASAN build?

Comment 2

[Tyson Smith [:tsmith]]

With an Linux 64bit ASan+Debug build I get:
Corrupt JPEG data: 1 extraneous bytes before marker 0xdd
Corrupt JPEG data: 1 extraneous bytes before marker 0xdd
JPEG decoding error:
Huffman table 0x0e was not defined

This could potentially a have different results on different platforms, build types, etc...

Comment 3

[DRC]

Well, this is an example of something that was already fixed but which the UBSan developers, for whatever reason, chose to re-break, and that frankly ticks me off.  Refer to:

https://github.com/libjpeg-turbo/libjpeg-turbo/commit/8e9cef2e6f5156c4b055a04a8f979b7291fc6b7a

Basically, this is another example of UBSan being overly pedantic.  The libjpeg code used to do this:

  jpeg_make_d_derived_tbl(cinfo, TRUE, dctbl,
                          & entropy->dc_derived_tbls[dctbl]);
  jpeg_make_d_derived_tbl(cinfo, FALSE, actbl,
                          & entropy->ac_derived_tbls[actbl]);

(Actually, the libjpeg code still does do that.  These issues were never fixed except in libjpeg-turbo.)

jpeg_make_d_derived_tbl() checks the value of the table index (dctbl or actbl) before it ever actually uses the pointer passed in the last argument, so there is no chance that it will overrun the array, but UBSan could be forgiven for thinking that it might.  So I changed the code to:

  pdtbl = entropy->dc_derived_tbls + dctbl;
  jpeg_make_d_derived_tbl(cinfo, TRUE, dctbl, pdtbl);
  pdtbl = entropy->ac_derived_tbls + actbl;
  jpeg_make_d_derived_tbl(cinfo, TRUE, actbl, pdtbl);

That apparently made UBSan happy for a while, but now it's complaining again.  So now I'm changing it to:

  pdtbl = (d_derived_tbl **)(entropy->dc_derived_tbls) + dctbl;
  jpeg_make_d_derived_tbl(cinfo, TRUE, dctbl, pdtbl);
  pdtbl = (d_derived_tbl **)(entropy->ac_derived_tbls) + actbl;
  jpeg_make_d_derived_tbl(cinfo, TRUE, actbl, pdtbl);

which seems to have once again silenced it.  Ugh.

Comment 4

[DRC]

NOTE: This is definitely not a security issue and never was.

Comment 5

[DRC]

Fix pushed:
https://github.com/libjpeg-turbo/libjpeg-turbo/commit/a1dd35680d40758f234f214cef4c53cd1bc4b34e