Buffer overflow in JPEG driver

[Mateusz Łoskot]

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!

[warmerdam]

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.

[Mateusz Łoskot]

I ported the tiny fix (r12054) to branches/1.4 (r12068).

[warmerdam]

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!