{{ message }}
fix: bounds check in Delete to prevent index out of range panic#280
Open
Yanhu007 wants to merge 1 commit intobuger:masterfrom
Open
fix: bounds check in Delete to prevent index out of range panic#280Yanhu007 wants to merge 1 commit intobuger:masterfrom
Yanhu007 wants to merge 1 commit intobuger:masterfrom
Conversation
|
@Yanhu007 hi! {
desc: "GO-2026-4514: malformed JSON without closing brace",
json: `{"a":1`,
path: []string{"a"},
data: `{`,
}, |
buger
added a commit
that referenced
this pull request
Apr 19, 2026
Bug fixes: - Fix Delete panic on truncated JSON (PR #280 class): tokenEnd returns len(data) as sentinel when no delimiter found, Delete used it as unchecked array index causing out-of-bounds panic on inputs like {"test":1 - Fix ArrayEach callback error swallowing: callback's err parameter was always nil despite the signature declaring it. Callers checking err in their callback had dead error handling code. Now the callback receives per-element parse errors before iteration stops. Dead code removal (all verified safe by MC/DC analysis + 60 targeted tests): - Remove tautological for-true loops (ArrayEach, Unescape, ObjectEach) - Remove dead tokenEnd end==-1 guard in getType (tokenEnd never returns -1) - Remove dead r<=basicMultilingualPlaneOffset in decodeUnicodeEscape (tautology) - Remove dead data[i]=='{' block-skip in EachKey (structurally unreachable) - Remove contradictory keys[level][0]!='[' in searchKeys (outer guard already checks ==) - Remove dead e!=nil in ArrayEach o==0 branch (Get offset=0 always means error) - Remove tautological ln>0 guard in findKeyStart (proven by prior nextToken check) Formal verification (ReqProof): - 92 requirements (7 stakeholder + 85 system) covering all API families - 18 obligation classes including truncated_at_value_boundary, sentinel_value_boundary, error_propagation, and truncated_escape_sequence - 100% requirement-level MC/DC (204/204 witness rows) - 100% code-level MC/DC (203/203 decisions, 244/244 conditions) - Kind2 realizability, consistency, and vacuity verification - Z3 data property proofs and behavioral implication proofs - 340 FLIP fixtures + 21 Z3 boundary fixtures - CI integration via probelabs/proof-action@v1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6 tasks
Owner
|
I have applied formal verification techniques to jsonparser using reqproof.com (e.g. it now close to being bug free is as much as possible, with some math proof) - #281 It supersede this PR, appreciate your review. |
Owner
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
4 participants
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 #274
Problem
Deleteaccessesdata[endOffset+tokEnd]without checking if the index is within bounds.tokenEnd()returnslen(data)when no terminator is found, soendOffset+tokEndcan exceed the slice length, causing a panic:Found by fuzzing.
Fix
Add a bounds check before accessing
data[endOffset+tokEnd]:All existing tests pass.