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

Issue in plist_free_data plist.c:185 #86

Closed
ghost opened this issue Jan 15, 2017 · 4 comments
Closed

Issue in plist_free_data plist.c:185 #86

ghost opened this issue Jan 15, 2017 · 4 comments

Comments

@ghost
Copy link

ghost commented Jan 15, 2017

=================================================================
==31671==ERROR: AddressSanitizer: SEGV on unknown address 0xfffffffffffffff1 (pc 0x7ff0c0b958f7 bp 0x7ffc50945b10 sp 0x7ffc50945280 T0)
    #0 0x7ff0c0b958f6  (/lib64/libasan.so.3+0x238f6)
    #1 0x7ff0c0c38acb in free (/lib64/libasan.so.3+0xc6acb)
    #2 0x7ff0c0964474 in plist_free_data /home/rs/dev/libplist/src/plist.c:185
    #3 0x7ff0c0964534 in plist_free_node /home/rs/dev/libplist/src/plist.c:205
    #4 0x7ff0c0964a26 in plist_free /home/rs/dev/libplist/src/plist.c:312
    #5 0x7ff0c0961540 in parse_dict_node /home/rs/dev/libplist/src/bplist.c:456
    #6 0x7ff0c0961540 in parse_bin_node /home/rs/dev/libplist/src/bplist.c:654
    #7 0x7ff0c0961bad in parse_bin_node_at_index /home/rs/dev/libplist/src/bplist.c:696
    #8 0x7ff0c096121a in parse_dict_node /home/rs/dev/libplist/src/bplist.c:439
    #9 0x7ff0c096121a in parse_bin_node /home/rs/dev/libplist/src/bplist.c:654
    #10 0x7ff0c0961bad in parse_bin_node_at_index /home/rs/dev/libplist/src/bplist.c:696
    #11 0x7ff0c0962418 in plist_from_bin /home/rs/dev/libplist/src/bplist.c:760
    #12 0x401624 in main /home/rs/dev/libplist/tools/plistutil.c:139
    #13 0x7ff0c038b400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    #14 0x400ec9 in _start (/home/rs/dev/libplist_asan/bin/plistutil+0x400ec9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libasan.so.3+0x238f6) 
==31671==ABORTING

Sample base64:
YnBsaXN0MDDRAgHTBAQEAAAAUUEQAVFCyrIACAALABIAFAAWAgEAAAAAAAAABQAAAAAAAAABAAAAAAAAABg=

@nikias
Copy link
Member

nikias commented Jan 16, 2017

Thanks for reporting Francisco :)
Now this is an interesting one. The error happens because there is a dictionary with a key node that is not a string type, but an integer. The value of the integer node is 1. Now in parse_dict_node() https://github.com/libimobiledevice/libplist/blob/master/src/bplist.c#L445 the type of the key node will be forced to be PLIST_KEY. In the same dictionary the value node is the object with index 0, but in the offset table, the offset is 0xCAB2 which will point outside of the actual plist data. Since this will make the creation of the value node fail (since the code detects the OOB access) plist_free() is called with the key node, which is actually an integer node but is forced to be of type PLIST_KEY. That will make the plist_free() (and in turn plist_free_node() then plist_free_data()) call free on data->buff which is not a valid pointer, causing the actual error.
Here comes the interesting part: Actually Apple allows key nodes to be a non-string. It can actually be any type. Only when converting to a different format this will produce an error. For example a binary plist with a dictionary with key(uint) 41, value(string) "B" (base64 YnBsaXN0MDDRAQIQQVFCCAsNAAAAAAAAAQEAAAAAAAAAAwAAAAAAAAAAAAAAAAAAAA8=), plutil will show this:

$ plutil -p s.bplist
{
  "<Error: Not a string: Is a: __NSCFNumber>" => "B"
}

For now, I will fix this with checking the actual type of the key node to be a string, since the rest of the library actually expects it to be a string when accessing items in the dictionary.

@ghost
Copy link
Author

ghost commented Jan 16, 2017

There is no need to give thanks :-), had pending to fuzz this library. Thank you Nikias!

nikias added a commit that referenced this issue Jan 16, 2017
As reported in #86, the binary plist parser would force the type of the
key node to be of type PLIST_KEY while the node might be of a different
i.e. non-string type. A following plist_free() might then call free() on
an invalid pointer; e.g. if the node is of type integer, its value would
be considered a pointer, and free() would cause an error.
We prevent this issue by disallowing non-string key nodes during parsing.
@nikias nikias closed this as completed Jan 18, 2017
@ghost
Copy link
Author

ghost commented Feb 2, 2017

It looks like someone has requested several CVEs for libplist including this issue CVE-2017-5836 in http://seclists.org/oss-sec/2017/q1/279

@nikias
Copy link
Member

nikias commented Feb 2, 2017

Yeah heh. I updated the comments for the other tickets. I will not update the commit messages to contain the CVEs but the NEWS file will be updated accordingly.

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

1 participant