Repository navigation
Fix heap out-of-bounds reads in the Android binary XML decoder ##crash - #26931
Merged
Merged
Conversation
Collaborator
|
I can confirm the four crashes with asan. The patch looks good! Thanks! |
Contributor
Author
|
appreciate the quick merge, and good to hear the asan repro lined up on your side too. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
bound the string pool and element reads in
r_axml_decodeby the size each chunk was allocated with, sincestring_count, the element name index andattribute_countall come straight from the file andpFAon a crafted AndroidManifest walks off both chunks.string_lookupindexedpool->offsets[]up to the file suppliedpool->string_count, whilepoolis onlyheader.sizebytes. an 80 byte binary XML with a 32 byte string pool chunk declaringstring_count=0x100000:a pool chunk shorter than 20 bytes gets its header fields read out of bounds the same way, a 16 byte start element chunk reads
attribute_countpast its own allocation, and the old attribute bound (count * sizeof (attribute_t) > element_size) ignored the 28 byte node header soattributes[]ran off the chunk too. the chunk size fields also stopped being truncated tout16, otherwise a string pool larger than 64KB would now be rejected instead of misparsed.test/unit/test_axml.cbuilds those four chunks plus a valid manifest in memory, so it aborts under the asan unit test job without the fix and passes with it.