fix: prevent stream decode buffer overflow panic (#424) - #603
Open
chiliec wants to merge 1 commit into
Open
Conversation
When decoding a JSON string from a stream, invalid or incomplete multi-byte UTF-8 characters are expanded to RuneError in place, growing s.buf via append and s.length, but leaving s.bufSize unchanged. When the buffer later needs to grow in readBuf, it doubled the stale s.bufSize, which could produce a new buffer smaller than the current buffer and smaller than s.length. The copy then truncated the data while s.length still referred to the old size, so scanning the buffer panicked with "index out of range". Resync s.bufSize to the real buffer length before doubling, and keep doubling until the new buffer can hold everything already read. Closes goccy#424.
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.
What
Closes #424.
Decoding certain JSON strings from a stream panicked:
Reproduced with a JSON string containing a long run of incomplete multi-byte UTF-8 lead bytes (
0xE2).Cause
When
stringBytesencounters invalid or incomplete multi-byte characters it replaces each withRuneErrorin place:This grows
s.buf(viaappend) ands.length, but never updatess.bufSize. Later, when the stream needs more data,readBufgrows the buffer from the stales.bufSize:At the panic, the real state was
len(s.buf)=1026,s.length=1282,s.bufSize=512. Doubling512 → 1024produced a buffer smaller than both the existing buffer ands.length. Thecopytruncated the data, buts.lengthstill referred to the old size, so the subsequent scans.buf[s.cursor+i](fori < s.length-s.cursor) ran off the end.Fix
In
readBuf, resyncs.bufSizeto the actual buffer length before doubling, and keep doubling until the new buffer can hold everything already read:The crafted input now returns a normal decode error instead of panicking.
Tests
Added
TestIssue424instream_test.go, reproducing the exact input.Verified RED→GREEN: without the fix,
TestIssue424panics withindex out of range [1025] with length 1024; with the fix it passes.Validation (real results, Go)
go test ./...— all packages pass (includingtest/cover).encoding/json, confirming the buffer growth does not corrupt data.gofmt -lclean. (go vetreports a pre-existingUnmarshalJSONsignature note ininternal/decoder/type.go, present on a cleanmastercheckout and unrelated to this change.)