fix: bounds-check nul sentinel in struct key escape scan (checkptr OOB) - #590
Open
momomuchu wants to merge 1 commit into
Open
fix: bounds-check nul sentinel in struct key escape scan (checkptr OOB)#590momomuchu wants to merge 1 commit into
momomuchu wants to merge 1 commit into
Conversation
decodeKeyCharByEscapedChar did not handle a dangling backslash at the end of the buffer. It treated the trailing nul sentinel as an unrecognized escaped char and returned a cursor past it, letting decodeKeyByBitmapUint8's raw char() read walk one byte past the allocation. Fatal under checkptr/-race on malformed input. Fixes goccy#577, goccy#575
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.
Fixes #577 (also fixes the #575 duplicate).
decodeKeyByBitmapUint8scans struct keys with raw unsafe.Pointerarithmetic via
char(). On a dangling backslash at the end of thebuffer (e.g.
{"hAs\),decodeKeyCharByEscapedCharhad no case for thetrailing nul sentinel, so it fell through to the unmatched-case
return nil, cursor, niland handed back a cursor still pointing atthe sentinel. The caller then advances one past it, and the next
char()reads one byte past the backing allocation. Under-race(which enables checkptr) that is a fatal, unrecoverable crash instead
of a decode error.
Handle nul as end of input and return
ErrUnexpectedEndOfJSON, thesame error the surrounding key decoder already returns for other
truncated-input cases. Confirmed the checkptr crash on master before
the change and its absence after, under
go test -race.Added TestIssue577 with the reported repro plus a few more
dangling-backslash variants, in the style of TestIssue429.