#1786 closed defect (fixed)
Buffer overflow in JPEG driver
Reported by: | Mateusz Łoskot | Owned by: | Mateusz Łoskot |
---|---|---|---|
Priority: | high | Milestone: | 1.4.3 |
Component: | GDAL_Raster | Version: | svn-trunk |
Severity: | critical | Keywords: | jpeg exif sprintf |
Cc: | warmerdam |
Description
Today, hobu and me fixed buildbot on the epimetheus slave and we found that autotest/gdrivers/jpeg.py test throws segmentation fault:
Running tests from gdrivers/jpeg.py TEST: jpeg_1 ... process killed by signal 11
No segmentation fault occurs on x86-32/64, neither on Linux nor on Windows. I generated backtrace using gdb on epimetheus:
(gdb) run ./jpeg.py Starting program: /usr/bin/python ./jpeg.py Reading symbols for shared libraries .................... done Reading symbols for shared libraries . done Reading symbols for shared libraries . done Reading symbols for shared libraries . done Reading symbols for shared libraries . done Reading symbols for shared libraries ............. done Reading symbols for shared libraries . done Reading symbols for shared libraries . done TEST: jpeg_1 ... JPEG: Magic: 0x4949 <little-endian> Version: 0x2a Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0xc0000000 0x90003040 in strcpy () (gdb) bt #0 0x90003040 in strcpy () #1 0x02131064 in JPGDataset::EXIFPrintData (this=0x185a800, pszData=0xbffdeb6a "02100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210"..., type=2, count=4, data=0xbffdeb66 "02100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210021002100210"...) at jpgdataset.cpp:247 #2 0x021323b8 in JPGDataset::EXIFExtractMetadata (this=0x31303032, fp=0x31303032, nOffset=825241650) at jpgdataset.cpp:569 #3 0x31303032 in ?? () Cannot access memory at address 0x31303032 Cannot access memory at address 0x3130303a
The line jpgdataset.cpp::247 consits of the following call:
JPGDataset::EXIFPrintData(char* pszData, GUInt16 type, GUInt32 count, unsigned char* data) { .... sprintf(pszData, "%s", data); // <--- 247 ... }
On the Power PC machine, here, sprintf tries to write beyond the buffer and the segmentation fault. I replaced (r12054) sprintf with snprintf call to limit writing to size of the pszData buffer (count value):
snprintf(pszData, count, "%s", data);
Technically, the bug seems to fixed now and no segmentation fault occurs anymore, but I'm not very familiar with JPEG driver, so I'm not sure if there is any logic error probable.
I'd be thankful if someone who knows JPEG driver could take a look at this fix and judge. Thanks!
Change History (3)
comment:1 by , 17 years ago
Cc: | added |
---|---|
Milestone: | → 1.4.3 |
Owner: | changed from | to
comment:2 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:3 by , 17 years ago
snprintf() does not behave as one might expect. It actually truncates the string at "n-1" characters on linux with a \0 terminator, but at "n" characters with no terminator on win32. I have replaced it with memcpy() and setting pszData[count] = '\0'; in r12085 and r12086 on trunk and branches/1.4. Test script updated accordingly.
Hopefully we are done with this bug!
I have found that there were serious bugs in the byte swapping logic for exif values stored entirely in the "data offset" value. I have restructured this code significantly (r12061) and updated the jpeg.py script to test more exif fields (r12060). The changes made in r12054 look fine.
Note, all these problems also existing in 1.4.x but for the time being I will avoid back patching. But the snprintf() change should be backfit to avoid crashes. Reassigning to mloskot to back apply the r12054 patch.