oc2: bound DAP lexer scan pointer against the strdup buffer end - #3375
Open
MarkLee131 wants to merge 1 commit into
Open
oc2: bound DAP lexer scan pointer against the strdup buffer end#3375MarkLee131 wants to merge 1 commit into
oc2: bound DAP lexer scan pointer against the strdup buffer end#3375MarkLee131 wants to merge 1 commit into
Conversation
daplex()'s main loop is
for(p=lexstate->next; token==0 && (c=*p); p++)
Several inner branches use `c = *(++p)` and exit when c == '\0', so p
ends AT the trailing NUL of the strdup'd input. The outer post-increment
then runs once more, leaving p ONE PAST the NUL, and the next (c=*p)
reads adjacent heap. ASan flags a 1-byte heap-buffer-overflow at
oc2/daplex.c:141 on a 3-byte malformed DAS/DDS body delivered via
nc_open() against a DAP server.
The same post-buffer pointer is then stored into lexstate->next, so
dap_parse_error()'s strlen(lexstate->next) (oc2/dapparse.c:454) walks
further into adjacent heap and copies what it finds into the stderr
"context:" line.
Compute the strdup buffer end once, bound the outer loop with p<=end,
and clamp p before assigning lexstate->next so the secondary strlen
also stays in-buffer.
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.
Summary
daplex()inoc2/daplex.cadvances its scan pointer past the trailing NUL of the strdup'd input buffer when it consumes a malformed token, then reads one byte past the buffer in the for-loop's(c=*p)condition. ASan flags a 1-byte heap-buffer-overflow READ atoc2/daplex.c:141on a 3-byte malformed DAS/DDS body. The same post-buffer pointer is later stored intolexstate->next, sodap_parse_error()'sstrlen(lexstate->next)(oc2/dapparse.c:454) walks further into adjacent heap and copies what it finds into the"context: ..."line printed to stderr.Reachable through the public API by opening any
dap://server/pathURL whose DAS/DDS response contains the trigger bytes --nc_open()->NCD2_open->dap_fetch->DAPparse->daplex.Reproducer
A Python mock DAP server returning the 3-byte body is enough.
mock_dap_server.py:poc.c:ASan output (current main):
Fix
Compute the strdup buffer's NUL position once in
daplex, bound the outer for-loop withp<=end, and clamppbefore assigning it back intolexstate->nextso the secondarystrlenindap_parse_erroralso stays in-buffer.Notes
oc2/dapparse.c:444-464isgit blamed to 2012-07-31, and the lexer loop has been similarly stable, so older release branches are likely affected too. I've only verified against currentmain.