Reject a hex escape whose digits are not hex digits - #3
Open
asp24 wants to merge 1 commit into
Open
Conversation
`{"a":"\u00 !"}` parses and produces a string holding a NUL, as do `\u00,-`, `\u+000`,
`\u0 00` and `\u00/.` - any of the four digit positions.
The digittoval table the assembly carries maps a byte to its value as a hex digit or to
-1, and its 0x00 to 0x2f entries are never written: the DATA block jumps from 0x038
straight to 0x070, so those forty-eight bytes are zero and read as the digit 0 instead
of as an error. The sixteen bytes from 0x20 to 0x2f are the space and most of the
punctuation, all legal inside a JSON string, so this is reachable from Parse and not
only from a unit test calling the routine directly. A byte such as 'Z' is outside the
gap and is rejected correctly, which is why the obvious test case never caught it.
The entries are spelled out now. A poisoned digit sign-extends into the high bits of the
code point, so the existing range check rejects it without a new branch. RFC 8259 wants
four hexadecimal digits, so no valid input changes: every document in testdata parses to
the same tape as before. Each of the seven cases added to parse_string_test.go was
confirmed to fail against the old table.
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
1 participant
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.
Candidate for
minio/simdjson-go. Cut from upstream master; based here on #2 only so that CI is green - on master the test suite panics until #2 lands.{"a":"\u00 !"}parses on upstream master today and yields a string holding a NUL. So do{"a":"\u00,-"}and{"a":"\u+000"}- any of the four digit positions can be a non-hex byte and the escape is still accepted.The
digittovaltable inparse_string_amd64.smaps a byte to its value as a hex digit or to -1, and entries 0x00-0x2f are never written: the DATA block jumps from 0x038 to 0x070, so those 48 bytes are zero and read as the digit 0 rather than as an error. The gap covers 0x20-0x2f - the space and most punctuation, all legal inside a JSON string - so it is reachable fromParse. A byte such asZsits outside the gap and is rejected correctly, which is why the obvious test case never caught this.The entries are spelled out now. A poisoned digit sign-extends into the high bits of the code point, so the existing range check rejects it without a new branch, and no valid input changes. Seven cases added to
parse_string_test.go, each confirmed to fail against the old table.