mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
549 stars 174 forks source link

[IT8] Out-of-bound array access on WriteDataFormat #373

Closed hghwng closed 1 year ago

hghwng commented 1 year ago

When fuzzing cmsIT8_load_fuzzer at commit 338cf94, I discovered a crash in WriteDataFormat. Specifically, it seems that cmsIT8GetProperty(it8, "NUMBER_OF_FIELDS") returns more fields than the file contains.

https://github.com/mm2/Little-CMS/blob/338cf946524714b2529750616d78581fce4b9b33/src/cmscgats.c#L1842-L1860

For easier debugging, you can reproduce the bug as follows:

$ echo ICIiCk5VTUJFUl9PRl9GSUVMRFMgNApTCkJFR0lOX0RBVEFfRk9STUFUCkkgUiBHIEIKRU5EX0RBVEFfRk9STUFUCkkKTlVNQkVSX09GX0ZJRUxEUyA4CgA= | base64 -d > input

$ ./cmsIT8_load_fuzzer input
StandaloneFuzzTargetMain: running 1 inputs
Running: input
AddressSanitizer:DEADLYSIGNAL
=================================================================
==146823==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000049 (pc 0x7f420090f35d bp 0x7fff5a3ccd40 sp 0x7fff5a3cc4e8 T0)
==146823==The signal is caused by a READ memory access.
==146823==Hint: address points to the zero page.
    #0 0x7f420090f35d  (/usr/lib/libc.so.6+0x15635d) (BuildId: 4a4bec3d95a1804443e852958fe59ed461135ce9)
    #1 0x564fd78d8e7c in __interceptor_strlen.part.0 asan_interceptors.cpp.o
    #2 0x564fd79807d3 in WriteStr cmscgats.c
    #3 0x564fd7977b2b in WriteDataFormat cmscgats.c
    #4 0x564fd7976d59 in cmsIT8SaveToFile (/root/build/lcms/bin-aubsan/cmsIT8_load_fuzzer+0x1cbd59) (BuildId: 56f8c6239bba48996be80daadee408876dcd0e91)
    #5 0x564fd79712ee in LLVMFuzzerTestOneInput (/root/build/lcms/bin-aubsan/cmsIT8_load_fuzzer+0x1c62ee) (BuildId: 56f8c6239bba48996be80daadee408876dcd0e91)
    #6 0x564fd79714c6 in main (/root/build/lcms/bin-aubsan/cmsIT8_load_fuzzer+0x1c64c6) (BuildId: 56f8c6239bba48996be80daadee408876dcd0e91)
    #7 0x7f42007dc78f  (/usr/lib/libc.so.6+0x2378f) (BuildId: 4a4bec3d95a1804443e852958fe59ed461135ce9)
    #8 0x7f42007dc849 in __libc_start_main (/usr/lib/libc.so.6+0x23849) (BuildId: 4a4bec3d95a1804443e852958fe59ed461135ce9)
    #9 0x564fd7872204 in _start (/root/build/lcms/bin-aubsan/cmsIT8_load_fuzzer+0xc7204) (BuildId: 56f8c6239bba48996be80daadee408876dcd0e91)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/usr/lib/libc.so.6+0x15635d) (BuildId: 4a4bec3d95a1804443e852958fe59ed461135ce9)
==146823==ABORTING
hghwng commented 1 year ago

Here's gdb's output, regarding nSamples and the array index i.

#2  0x0000555555640667 in WriteDataFormat (fp=<optimized out>, it8=0x7ffff7b18010) at cmscgats.c:1855
1855                  WriteStr(fp, t->DataFormat[i]);
(gdb) p i
$1 = 5
(gdb) p *t
$2 = {SheetType = "\"\"\000TS.17", '\000' <repeats 1015 times>, nSamples = 4, nPatches = 0, SampleID = 0, HeaderList = 0x55555f0bfb30, DataFormat = 0x55555f0bfbf8, Data = 0x0}
mm2 commented 1 year ago

Thanks for reporting. Should be solved in e85c64ec86b9025c696f707d820ba28e3e732e70 Please note this affects only to CGATS parser, which is currently only used by command line sample tools. So, no exploits can be done by that. The ICC parsing and handling does not use this code in any way.

hghwng commented 1 year ago

Thanks for the patch — I can confirm that this problem has been fixed, thus I'm closing this issue. Thank you for maintaining LCMS — I'm using it on my Linux laptop right now 🤗