Fix out of bounds write in scanner RPG rule#77
Open
Alearner12 wants to merge 1 commit into
Open
Conversation
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.
Bug
yylex(cue_scanner.l:128) contains an off-by-one error when parsingREM REPLAYGAIN_*fields that causes a 1-byte out-of-bounds write. When the scanner enters the<RPG>state, it copies the matched characters into the globalyy_buffer(which is defined asPARSER_BUFFER/ 1024 bytes).The code currently bounds the
strncpybysizeof(yy_buffer), but then sets the null terminator at indexsizeof(yy_buffer):yylval.sval[(yyleng > sizeof(yy_buffer) ? sizeof(yy_buffer) : yyleng)] = '\0';If the input string is exactly 1024 characters or longer, this writes
\0toyylval.sval[1024], which is 1 byte past the end of the 1024-byteyy_buffer. Becauseyy_bufferis a global array, this triggers a global buffer overflow, corrupting the adjacentyytextvariable in memory. ASan catches this cleanly.Reproduction
Create a minimal
.cuefile with a ReplayGain tag containing a value of 1024 bytes or more.Build libcue with
gcc -O1 -g -fsanitize=addressand run a small harness that callscue_parse_string()on the payload:Running this under ASan predictably traps:
Fix
The fix is to just use the exact same truncation logic that the
<NAME>rule (around line 46) already uses. By changing the string copy length and null termination index tosizeof(yy_buffer) - 1, we ensure there's always space for the null byte. This safely truncates any overly long ReplayGain values to fit within the buffer limits.Verification
Tested the exact same 1024-byte and 1025-byte PoC payloads against the patched code with ASan enabled:
All previously-OOB lengths are now safely truncated, and in-bounds values are processed just like before. No other existing logic or parsing rules are affected since the change is isolated to the RPG state handler.