Menu
▾
▴
Re: [Libmtp-discuss] An interger overflow bug in libmtp (Version: 1.1.12)
|
From: Marcus M. <mei...@su...> - 2017-03-16 15:01:06
|
Hi,
Thanks for the report!
I have added
if (prop_count >= INT_MAX/sizeof(MTPProperties)) {
ptp_debug (params ,"prop_count %d is too large", prop_count);
return 0;
}
to the function.
Ciao, Marcus
On Thu, Mar 16, 2017 at 01:21:40PM +0800, wyk...@gm... wrote:
> Hi，I find an interger overflow bug in libmtp:
>
> OverFlow Point:
> static inline int ptp_unpack_OPL (PTPParams *params, unsigned char* data, MTPProperties **pprops, unsigned int len)
> |
> |->uint32_t prop_count = dtoh32a(data); //read an uint32_t number from data
> //...
> props = malloc(prop_count * sizeof(MTPProperties)); //prop_count can be very large, if multiplied by sizeof(MTPProperties), overflow occurs
> //...
> for (i = 0; i < prop_count; i++) {
> props[i].ObjectHandle = dtoh32a(data);
> data += sizeof(uint32_t);
> len -= sizeof(uint32_t);
>
> props[i].property = dtoh16a(data);
> data += sizeof(uint16_t);
> len -= sizeof(uint16_t);
>
> props[i].datatype = dtoh16a(data);
> data += sizeof(uint16_t);
> len -= sizeof(uint16_t);
>
> Trigger:
> In example/track.c file, from main function, will go to ptp_unpack_OPL function:
>
> int main (int argc, char **argv)
> |
> |->dump_tracks(device, storage->id, 0);
> |
> |->files = LIBMTP_Get_Files_And_Folders(device, storageid, leaf);
> |
> |->file = LIBMTP_Get_Filemetadata(device, currentHandles.Handler[i]);
> |
> |->ret = ptp_object_want(params, fileid, PTPOBJECT_OBJECTINFO_LOADED|PTPOBJECT_MTPPROPLIST_LOADED, &ob);
> |
> |->if (params->device_flags & DEVICE_FLAG_BROKEN_MTPGETOBJPROPLIST) { //Through a android phone device, this check may be goto fallback, but I think there will some other ways to bypass.
> want &= ~PTPOBJECT_MTPPROPLIST_LOADED;
> goto fallback;
> }
> //...
> ret = ptp_mtp_getobjectproplist_single (params, handle, &props, &nrofprops);
> |
> |->CHECK_PTP_RC(ptp_transaction(params, &ptp, PTP_DP_GETDATA, 0, &data, &size)); //get data by PTP_DP_GETDATA transaction, and data is from portable device such as phone, so it's content is controllable.
> |
> |->...
>
> *nrofprops = ptp_unpack_OPL(params, data, props, size);
> |
> |->uint32_t prop_count = dtoh32a(data);
> //...
> props = malloc(prop_count * sizeof(MTPProperties));
> for (i = 0; i < prop_count; i++) { //This for loop will copy data to props, will lead to out-of-bounds write
> //...
>
> Crash log:
> Attempting to connect device(s)
> Device 0 (VID=18d1 and PID=4ee2) is a Google Inc (for LG Electronics/Samsung) Nexus 4/5/7/10 (MTP+ADB).
> mtp-tracks: Successfully connected
> Android device detected, assigning default bug flags
> Friendly name: (NULL)
> [*]dump_tracks
> [*]LIBMTP_Get_Files_And_Folders
> [*]LIBMTP_Get_Filemetadata
> [*]function ptp_object_want
> [*]ptp_mtp_getobjectproplist_single
> [*]size:221
> 010000800100000001dc0600010001000100000002dc040001300100000003dc040000000100000004dc080000000000000000000100000007dcffff064d00750073006900630000000100000009dcffff103100390037003000310031003200330054003200320033003000310032000000010000000bdc0600000000000100000041dc0a00010000000100010000000000000000000100000044dcffff064d007500730069006300000001000000e0dcffff00010000004edcffff103100390036003900310032003300310054003100390030003000300030000000
> [*]prop_count:2147483649
> [*]sizeof(MTPProperties):24
> [*]loop
> [*]len:217
> [*]len:205
> [*]len:195
> [*]len:185
> [*]len:169
> [*]len:148
> [*]len:107
> [*]len:95
> [*]len:71
> [*]len:50
> [*]len:41
> [*]len:0
> [*]LIBMTP_Get_Filemetadata
> [*]function ptp_object_want
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff75ac204 in ?? () from /lib/x86_64-linux-gnu/libusb-0.1.so.4
> (gdb) bt
> #0 0x00007ffff75ac204 in ?? () from /lib/x86_64-linux-gnu/libusb-0.1.so.4
> #1 0x00007ffff7bac31d in ptp_write_func.isra.9 () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #2 0x00007ffff7bad8dd in ptp_usb_sendreq () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #3 0x00007ffff7b9cb17 in ptp_transaction_new () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #4 0x00007ffff7b9cd08 in ptp_transaction () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #5 0x00007ffff7b9ea09 in ptp_getobjectinfo () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #6 0x00007ffff7baaca4 in ptp_object_want () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #7 0x00007ffff7b93cd1 in LIBMTP_Get_Filemetadata () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #8 0x00007ffff7b93f84 in LIBMTP_Get_Files_And_Folders () from /home/ubuntu/Android/Fuzz/libmtp-1.1.12/src/.libs/libmtp.so.9
> #9 0x00000000004010b9 in dump_tracks () at tracks.c:89
> #10 0x0000000000400ec4 in main () at tracks.c:185
>
> Fix:
> Add check in ptp_unpack_OPL function:
>
> + if (prop_count >= UINT_MAX/sizeof(MTPProperties))
> + return 0;
> props = malloc(prop_count * sizeof(MTPProperties));
>
>
>
> wyk...@gm...
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Libmtp-discuss mailing list
> Lib...@li...
> https://lists.sourceforge.net/lists/listinfo/libmtp-discuss
|
