Properly zero initialise the ogg and vorbis state structs - CVE-2017-15185 #359
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 prevents things from exploding in flames if an error occurs and the code tries to unwind before the codec and container initialiser functions can all be called. It fixes the second issue indicated in CVE-2017-11333, which isn't the fault of libvorbis, it's caused by us passing junk data to vorbis_block_clear() when an invalid file is detected and we bail out before vorbis_block_init() gets called. Ideally, we should simplify all of this and get rid of most of the malloc farm there by embedding the needed structs in splt_ogg_state (instead of pointers to them), then just do a single malloc and memset for the whole lot - but that would be a much more intrusive change, so for now just ensure the allocated memory is all safely zeroed in the simplest manner. CVE-2017-15185
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 is imported from the Debian package:
This prevents things from exploding in flames if an error occurs and the
code tries to unwind before the codec and container initialiser functions
can all be called. It fixes the second issue indicated in CVE-2017-11333,
which isn't the fault of libvorbis, it's caused by us passing junk data
to vorbis_block_clear() when an invalid file is detected and we bail out
before vorbis_block_init() gets called.
Ideally, we should simplify all of this and get rid of most of the malloc
farm there by embedding the needed structs in splt_ogg_state (instead of
pointers to them), then just do a single malloc and memset for the whole
lot - but that would be a much more intrusive change, so for now just
ensure the allocated memory is all safely zeroed in the simplest manner.