fix(protobuf): reject malformed varints across values and framing#2258
Open
samchon wants to merge 3 commits into
Open
fix(protobuf): reject malformed varints across values and framing#2258samchon wants to merge 3 commits into
samchon wants to merge 3 commits into
Conversation
Claim issue #2256 before implementation so one batch owns value, tag, length-prefix, wrapper, and independent-oracle consequences of the shared reader boundary. Constraint: Campaign pull requests cannot change versions, tags, releases, or publication state. Rejected: Limit the fix to declared integer fields | uint32 also owns tags and every length prefix. Confidence: high Scope-risk: moderate Directive: Enforce the wire invariant without prescribing reader control flow and retain every valid boundary control. Tested: Independent value/tag/length reproductions and official Go oracle critic review passed on ce88a88. Not-tested: Product implementation and integration gates are intentionally pending. Related: #2256 #2135 #2139
All value and framing consumers share the tenth wire byte, so enforce its single legal payload bit in one reader boundary. Permanent runtime, generated wrapper, and official protowire regressions keep malformed input from becoming success-shaped decoded output. Constraint: Campaign implementation freezes package versions, tags, and publication state Rejected: Mask or discard the tenth byte | preserves the silent truncation defect Rejected: Guard generated decoders only | direct readers, tags, lengths, and unknown fields share the runtime owner Confidence: high Scope-risk: moderate Reversibility: clean Directive: Preserve legal ten-byte maxima and non-canonical in-range encodings when changing varint parsing Tested: Focused reader/decode regressions, official Go oracle, mutation proof, full schema workspace, feature identity, pnpm build, pnpm test, tagged native Go tests, native/test go vet Not-tested: Prospective-merge and exact-merge gates; independent verifier owns those campaign stages Related: #2256
The Go oracle test exercised protowire alone, so it stayed green with the product fix fully reverted while its name promised typia's own bounds. Every byte string and verdict now lives in one corpus both languages read: the Go test proves the corpus still matches the official parser, and the TypeScript regressions read the same rows through typia's reader and generated decoders, so neither side can transcribe a verdict the other has not seen. The tenth byte also carried one message for two faults. A continuing tenth byte would occupy an eleventh, but an excess payload bit terminates inside ten bytes and exceeds no length, so it now reports the 64-bit domain it left. Acceptance is unchanged: the same two tenth-byte values pass as before. Constraint: Campaign implementation freezes package versions, tags, and publication state Rejected: Run typia's reader from Go | the one Go-to-node harness stubs every typia/lib/internal import away, and a built lib is no prerequisite of the Go suites Rejected: Keep the corpus in each test | hand transcription is what let the oracle and the regressions drift Rejected: Keep one message for both tenth-byte faults | a ten-byte payload terminates within ten bytes and exceeds no length, so one message could only name one of the two faults truthfully Confidence: high Scope-risk: low Reversibility: clean Directive: Read the shared corpus instead of writing a varint byte string into a new test Tested: Isolated ttsx cache; corpus oracle test with two corpus-drift controls, focused reader and decode regressions, all 18 protobuf cases, 178-case schema workspace, feature identity, go vet over the test module, and a mutation proof with an executed-code probe Not-tested: Prospective-merge and exact-merge gates; the lead owns those campaign stages Related: #2256
samchon
marked this pull request as ready for review
July 19, 2026 15:34
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.
Closes #2256.
A malformed Protocol Buffer varint could become success-shaped decoded output. All value and framing consumers share the tenth wire byte, so the fix enforces its single legal payload bit in one reader boundary in
_ProtobufReader.ts, and corrects the #2139 reader regression that had frozen ten-continuation acceptance.Verification
Independently verified by an agent that did not write the implementation and built its own oracle rather than reusing the batch's test material.
protowirev1.34.2 — 885 byte strings x 8 reader entry points{0x00, 0x01}, bit-identical to the oracle; base accepted all 256Error— nevernull, neversuccess: falseMutation proof: reverting only
_ProtobufReader.tsfails exactly the three coupled regressions while all fifteen adjacent protobuf controls pass. Gates at the head:pnpm build, 18 protobuf cases, 178-casetest-typia-schema, feature identity,go vetin both trees, and tagged native../native/...(28 packages, 0 failures).The changed #2139 expectation is a correction, not a laundered regression
A previously merged test asserted that typia must accept
0x80 x10, 0x01as0.protowirerejects that input withn = -3. The old expectation was wrong; the new one matches both the wire specification and the oracle. Adjudicated independently against the specification rather than against this pull request's narrative.Second commit: the oracle test could not fail
Independent verification found the first commit's permanent
TestProtobufVarintBoundsOraclepassed with the product fix fully reverted — it exercisedprotowirealone, never called typia, and its agreement with the TypeScript regressions was hand transcription. A permanent test that cannot fail when the work regresses is a false green, so the batch was returned for a test-asset remediation.Every byte string and verdict now lives in one
packages/typia/test/protobuf_varint_corpus.jsonthat the Go test and both TypeScript regressions read. The Go test is renamed to state that it pins the official parser's verdicts only. Verified by mutating the corpus seven ways — value, consumed length, fault relabelling, emptied entries, unknown class, a fabricated contradictory row, and an accepted/rejected flip — with both halves reacting to every mutation.The same commit splits one error message that had described two different faults: a continuing tenth byte keeps
varint exceeds 10 bytes., while a terminating tenth byte carrying payload above bit 0 now reportsvarint exceeds 64 bits.Acceptance is byte-identical — proven by re-running the full 7080-cell differential across both heads: 0 acceptance differences, 0 value differences, 2088 message-only differences. No website page, documentation, example, or tracked test outside this batch pinned the old string.Out of scope, recorded for the next discovery round
Two verified, user-visible defects present identically on the base commit and on this fix, outside this issue's published invariant: length prefixes and field tags are silently truncated to 32 bits, so a field declaring 4,294,967,301 bytes reads five, and field 536870913 aliases onto field 1. A third consumer of the same truncation,
bool(), decodes from the low 32 bits only. None is locked in by the new tests, so the next round can decide the contract rather than inherit a snapshot.Campaign constraints
No package version, tag, release, publication, or
packages/interface/srcchange.pnpm formatwas not run.