Dex fixes #1728
Merged
Dex fixes #1728
Conversation
This file contains 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
This commit fixes a few crashes in the dex module. There are actually three of them: The first is incorrect usage of "struct_fits_in_dex" caused by passing "sizeof(code_item_t)" instead of just "code_item_t" as the third argument. In the test case the pointer for code_item started in the bounds of the dex but only the first 8 bytes were within bounds, and since "sizeof(sizeof(code_item_t))" is less than 8 the check was passing. The fix here is to pass just the struct type as the third argument. The second crash was an off-by-one error when parsing a string. The check ensured the string fits in the dex but was not including an extra byte which was copied in the call to set_sized_string. Just like before, this was a case of a string falling right on the end of a dex file. The third crash was due to a missing "struct_fits_in_dex" check. We ended up with a pointer to a map_item_t which was off the ends of the dex bounds. With this commit all the test cases provided in the report are now passing. I did a quick sweep of the module to make sure there were no other cases where we were incorrectly using "struct_fits_in_dex" and didn't find any. These were all documented at a private report via huntr.dev (https://huntr.dev/bounties/007a7784-c211-4847-9cc3-aec38e7d5157/) Found by @sudhackar. Fixes VirusTotal#1726.
|
Forgot to mention, I fixed some compiler warnings in a previous commit on this branch. Probably should have sent that as a separate PR, sorry for the noise. |
plusvic
approved these changes
Jun 15, 2022
|
I confirmed that all the provided test cases in the report are no longer causing crashes under asan. |
|
@JamieSlome I've updated the report. We'll wait for the reporter's confirmation that he doesn't reproduce the issues anymore with the latest changes. |
|
@plusvic - thank you ❤️ |
plusvic
pushed a commit
that referenced
this pull request
Jun 30, 2022
* Fix compiler warnings with dex debug mode. * Fix crashes in dex module. This commit fixes a few crashes in the dex module. There are actually three of them: The first is incorrect usage of "struct_fits_in_dex" caused by passing "sizeof(code_item_t)" instead of just "code_item_t" as the third argument. In the test case the pointer for code_item started in the bounds of the dex but only the first 8 bytes were within bounds, and since "sizeof(sizeof(code_item_t))" is less than 8 the check was passing. The fix here is to pass just the struct type as the third argument. The second crash was an off-by-one error when parsing a string. The check ensured the string fits in the dex but was not including an extra byte which was copied in the call to set_sized_string. Just like before, this was a case of a string falling right on the end of a dex file. The third crash was due to a missing "struct_fits_in_dex" check. We ended up with a pointer to a map_item_t which was off the ends of the dex bounds. With this commit all the test cases provided in the report are now passing. I did a quick sweep of the module to make sure there were no other cases where we were incorrectly using "struct_fits_in_dex" and didn't find any. These were all documented at a private report via huntr.dev (https://huntr.dev/bounties/007a7784-c211-4847-9cc3-aec38e7d5157/) Found by @sudhackar. Fixes #1726.
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.
This fixes the crashes reported on huntr.dev. While there were 5 test cases reported there were only three distinct bugs triggered.
I'll take a look at #950 and #951 tomorrow night but there's a good chance they are already fixed.