Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

heap-buffer-overflow in bplist.c:733 #103

Closed
zhunki opened this issue Apr 18, 2017 · 5 comments
Closed

heap-buffer-overflow in bplist.c:733 #103

zhunki opened this issue Apr 18, 2017 · 5 comments

Comments

@zhunki
Copy link
Contributor

zhunki commented Apr 18, 2017

==26853== ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb58018c8 at pc 0x80665b8 bp 0xbfb96b58 sp 0xbfb96b4c
READ of size 8 at 0xb58018c8 thread T0
    #0 0x80665b7 in parse_bin_node_at_index /home/b/libplist/src/bplist.c:733
    #1 0x8069720 in plist_from_bin /home/b/libplist/src/bplist.c:857
    #2 0x804bbab in main /home/b/libplist/tools/plistutil.c:150
    #3 0xb5fe6a82 (/lib/i386-linux-gnu/libc.so.6+0x19a82)
    #4 0x804c805 in _start (/home/b/libplist/tools/plistutil+0x804c805)
0xb58018c8 is located 24 bytes to the left of 0-byte region [0xb58018e0,0xb58018e0)
==26853== AddressSanitizer CHECK failed: ../../../../src/libsanitizer/asan/asan_allocator2.cc:216 "((id)) != (0)" (0x0, 0x0)
    #0 0xb61aa4b2 (/usr/lib/i386-linux-gnu/libasan.so.0+0x124b2)
    #1 0xb61b30dc (/usr/lib/i386-linux-gnu/libasan.so.0+0x1b0dc)
    #2 0xb619bf1e (/usr/lib/i386-linux-gnu/libasan.so.0+0x3f1e)
    #3 0xb61b06e3 (/usr/lib/i386-linux-gnu/libasan.so.0+0x186e3)
    #4 0xb61b1b8f (/usr/lib/i386-linux-gnu/libasan.so.0+0x19b8f)
    #5 0xb61aa9ee (/usr/lib/i386-linux-gnu/libasan.so.0+0x129ee)
    #6 0x80665b7 in parse_bin_node_at_index /home/b/libplist/src/bplist.c:733
    #7 0x8069720 in plist_from_bin /home/b/libplist/src/bplist.c:857
    #8 0x804bbab in main /home/b/libplist/tools/plistutil.c:150
    #9 0xb5fe6a82 (/lib/i386-linux-gnu/libc.so.6+0x19a82)
    #10 0x804c805 in _start (/home/b/libplist/tools/plistutil+0x804c805)
Aborted

bplist_c_733.txt

@nikias
Copy link
Member

nikias commented Apr 18, 2017

I cannot reproduce it. This check prevents further processing for me:
https://github.com/libimobiledevice/libplist/blob/master/src/bplist.c#L837

    if ((uint64_t)offset_table + num_objects * offset_size > (uint64_t)end_data) {
        PLIST_BIN_ERR("offset table points outside of valid range\n");
        return;
    }

It is supposed to prevent that the number of objects combined with the offset size will point outside of the plist data. The sample you provided has number of objects = 0x8000000000000003 and offset size = 0xFF. Maybe your compiler reduces the sizes to 32 bit? Strangely enough, even then it would point outside of the plist data...

@nikias
Copy link
Member

nikias commented Apr 18, 2017

@zhunki can you use this patch and paste the output:

diff --git a/src/bplist.c b/src/bplist.c
index eede7a7..07084f6 100644
--- a/src/bplist.c
+++ b/src/bplist.c
@@ -834,6 +834,11 @@ PLIST_API void plist_from_bin(const char *plist_bin, uint32_t length, plist_t *
         return;
     }

+printf("offset_table: 0x%llx\n", (uint64_t)offset_table);
+printf("num_objects: 0x%llx\n", num_objects);
+printf("offset_size: 0x%x\n", offset_size);
+printf("end_data: 0x%llx\n", (uint64_t)end_data);
+printf("range check: 0x%llx > 0x%llx\n", (uint64_t)offset_table + num_objects * offset_size, (uint64_t)end_data);
     if ((uint64_t)offset_table + num_objects * offset_size > (uint64_t)end_data) {
         PLIST_BIN_ERR("offset table points outside of valid range\n");
         return;

@zhunki
Copy link
Contributor Author

zhunki commented Apr 19, 2017

@nikias the result is as follows
offset_table: 0xffffffffb58017d1
num_objects: 0x8000000000000003
offset_size: 0xff
end_data: 0xffffffffb58017d4
range check: 0x7fffffffb5801ace > 0xffffffffb58017d4

ps. 0x8000000000000003*0xff=0x‭80000000000002FD
integer overflowed.
0x‭80000000000002FD+0xffffffffb58017d1=‭0x7FFFFFFFB5801ACE‬
integer overflowed again.

I'm using gcc, maybe your conpiler is defferent and can handle such case?

@nikias
Copy link
Member

nikias commented Apr 19, 2017

@zhunki should be fixed with my latest commit: fdebf8b

@zhunki
Copy link
Contributor Author

zhunki commented Apr 20, 2017

@nikias sure. well done.

@nikias nikias closed this as completed Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants