Skip to content

THRIFT-6071: Validate container size fits int32 range before narrowing conversion in TSimpleJSONProtocol#3601

Open
Jens-G wants to merge 1 commit into
apache:masterfrom
Jens-G:THRIFT-6071
Open

THRIFT-6071: Validate container size fits int32 range before narrowing conversion in TSimpleJSONProtocol#3601
Jens-G wants to merge 1 commit into
apache:masterfrom
Jens-G:THRIFT-6071

Conversation

@Jens-G

@Jens-G Jens-G commented Jun 17, 2026

Copy link
Copy Markdown
Member

Summary

ParseElemListBegin reads the list/set element count from the wire as int64 via ParseI64, but passes int32(nSize) to checkSizeForProtocol. Values larger than math.MaxInt32 are silently narrowed before the check; the truncated value may pass while the full int64 is stored in size and returned to callers.

The fix adds an explicit range check (nSize > math.MaxInt32) before the narrowing cast, returning SIZE_LIMIT immediately. The existing checkSizeForProtocol call is retained for the normal range.

ReadListBegin and ReadSetBegin both delegate to ParseElemListBegin and are covered by this change. binary_protocol, compact_protocol, and json_protocol read container sizes as int32 directly and are not affected.

Related: PR #3599 addresses an analogous issue in ReadMapBegin; the same guard should be added there once that fix lands.

Test plan

  • TestReadSimpleJSONProtocolListBeginSizeOverflow — new test covering list and set; verifies SIZE_LIMIT is returned for a declared size of 1<<32 + 1 (which int32 truncation would reduce to 1)
  • Full Go library test suite: go test ./... passes

🤖 Generated with Claude Code

…g conversion in TSimpleJSONProtocol

Client: go

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Jens-G Jens-G requested a review from fishy as a code owner June 17, 2026 15:13
@mergeable mergeable Bot added the golang Pull requests that update Go code label Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

golang Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants