fix(bytesconv): detect integer overflow before multiplication in ParseUintBuf - #1538
Open
GerardGao wants to merge 1 commit into
Open
fix(bytesconv): detect integer overflow before multiplication in ParseUintBuf#1538GerardGao wants to merge 1 commit into
GerardGao wants to merge 1 commit into
Conversation
…eUintBuf ParseUintBuf checked for overflow after computing 10*v + k. When 10*v itself overflows, the wrapped result can still be greater than v, so the check silently passed and the function returned a wrapped value instead of errTooLongInt. Move the check before the multiplication so out-of-range input is rejected consistently. Reuses the existing errTooLongInt and keeps the function signature unchanged. Add the previously missed inputs to TestParseUintError.
GerardGao
force-pushed
the
fix/parseuint-overflow-detection
branch
from
August 25, 2026 13:30
c7e55c0 to
a60cc56
Compare
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 this PR does
ParseUintBufchecks for overflow after the multiplication:When
10*vitself overflows, the wrapped result can still be greater thanv,so the check silently passes and the function returns a wrapped value instead of
errTooLongInt.This PR moves the check before the multiplication.
Reproduce
ParseUintaccepts out-of-range input and returns a wrapped value:210000000000000000002553255926290448384,err == nilerrTooLongInt250000000000000000006553255926290448384,err == nilerrTooLongIntScanning all 90 twenty-digit inputs of the form
dd000000000000000000, 18 ofthem are accepted although they exceed
math.MaxInt.Note the inconsistency:
18446744073709551616is correctly rejected today,while the numerically smaller
21000000000000000000is accepted.Impact
ParseUintBufbacksprotocol.ParseContentLength, so the value comes from anuntrusted request header. Parsing a real request through
ReadHeader:ParseUintis also used for theRangeheader (pkg/app/fs.go) and the cookiemax-ageattribute (pkg/protocol/cookie.go).Notes
math.MaxIntso the bound follows the platform word size; verifiedGOARCH=386andGOARCH=armstill build.errTooLongInt; no new error value and no signature change.9223372036854775807is still accepted, covered by theexisting
TestParseUint.Verification
TestParseUintError; they fail beforethis change and pass after it.
go test ./...passes.gofmt -landgo vetare clean.I could not open an issue first as suggested by
CONTRIBUTING.md, because issuecreation is restricted in this repository, so the full reproduction is included
above. Happy to move this to an issue or another channel if you prefer.