Address infinite loops while reading ID3v2 tags. #48
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
The `Buffer.read()` call will infinitely loop if the number of bytes remaining in the file is less than the requested size. A large number of `read()` calls in `ID3v2Parser` are at risk of infinitely looping on malformed files. I'm changing `Buffer` to throw an exception when it detects there are no more bytes to read from the file. There is a more egregious case with the potential `Xing` header. The parser blindly reads 1500 bytes to search for the header, but there is no guarantee (even in a well-formed file) that there will be 1500 more bytes remaining to be read. I'm introducing a `readAtMost` method to `Buffer` which will handle cases where there are fewer than `size` bytes remaining in the file. I'm also updating some of the logic which is checking for the start of the `Xing` header to be safer, checking to ensure that there are enough bytes remaining to parse the data from this header.
|
Thank you for your PR! I'm going to review it soon. |
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.
The
Buffer.read()call will infinitely loop if the number of bytes remaining in the file is less than the requested size. A large number ofread()calls inID3v2Parserare at risk of infinitely looping on malformed files. I'm changingBufferto throw an exception when it detects there are no more bytes to read from the file.There is a more egregious case with the potential
Xingheader. The parser blindly reads 1500 bytes to search for the header, but there is no guarantee (even in a well-formed file) that there will be 1500 more bytes remaining to be read. I'm introducing areadAtMostmethod toBufferwhich will handle cases where there are fewer thansizebytes remaining in the file.I'm also updating some of the logic which is checking for the start of the
Xingheader to be safer, checking to ensure that there are enough bytes remaining to parse the data from this header.This is a partial fix for #47. I only looked at the first (mp3) case. The second (ogg) case might be unrelated.