avoid out-of-bounds read on empty ELF note name - #502
Open
fr-manvi wants to merge 1 commit into
Open
Conversation
Author
|
any update? |
1 similar comment
Author
|
any update? |
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.
GetBuildId walks the notes in every SHT_NOTE section, and each note's name comes from NoteIter::Next as name_ = StrictSubstr(remaining_, 0, note.n_namesz). The n_namesz field is read straight out of the note header, so it is attacker-controlled just like the rest of the object. When a note carries n_namesz == 0 the name_ view is empty, and the very next line checks name_[name_.size() - 1] to trim a trailing NUL, so name_.size() - 1 wraps to SIZE_MAX and the operator[] reads outside the view. I hit it feeding a crafted ET_DYN with a single empty-name note; a build with libc++ hardening traps in NoteIter::Next, and it is on the default path since GetBuildId runs for every non-object input. Guarding the trim with !name_.empty() closes it while leaving the normal case (a name ending in NUL) unchanged. Added a yaml2obj regression alongside the other elf tests.