Safer sections iteration #29
Open
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
assafnativ
force-pushed
the
nativ/fix_null_deref
branch
from
16527d1 to
a437584
Compare
|
+1 for this |
aclements
reviewed
Jul 29, 2020
| @@ -103,8 +103,14 @@ elf::elf(const std::shared_ptr<loader> &l) | |||
| // Load sections | |||
| const void *sec_data = l->load(m->hdr.shoff, | |||
| m->hdr.shentsize * m->hdr.shnum); | |||
| if (NULL == sec_data) { | |||
There was a problem hiding this comment.
loader.load is not allowed to return NULL (it's supposed to throw an exception if there's a problem) and the mmap loader implementation doesn't return NULL, so I'm afraid I don't understand what this change is accomplishing.
| for (unsigned i = 0; i < m->hdr.shnum; i++) { | ||
| const void *sec = ((const char*)sec_data) + i * m->hdr.shentsize; | ||
| if (NULL == sec) { |
There was a problem hiding this comment.
It doesn't make sense for sec to be NULL here, since it's a result from a pointer offset from sec_data. In practice the only way this can happen is if sec_data itself is NULL, but then we shouldn't have even done the pointer arithmetic.
| @@ -103,8 +103,14 @@ elf::elf(const std::shared_ptr<loader> &l) | |||
| // Load sections | |||
| const void *sec_data = l->load(m->hdr.shoff, | |||
| m->hdr.shentsize * m->hdr.shnum); | |||
| if (NULL == sec_data) { | |||
There was a problem hiding this comment.
Please follow the coding conventions of the surrounding code, which uses forward comparison: sec_data == NULL.
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.
File with the hash of ee4b726d2601b084e02f4fb7411a26054fa8a1053f011d3ee9d5a422d93f342c was crashing this parser.