fix(xdr): bound decimal string length before BigInt parse in JSON decode - #1581
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Bounds SEP-51 integer JSON strings before BigInt() parsing to avoid unnecessary work on oversized input.
Changes:
- Adds a width-based decimal-string budget validator.
- Applies validation to 64-, 128-, and 256-bit JSON decoders.
- Adds regression and boundary coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/xdr/values/bigint-parts.ts |
Adds digit-budget validation. |
src/xdr/values/to-json.ts |
Validates integer strings before parsing. |
test/unit/xdr/xdr_json_bigint_digit_budget.test.ts |
Tests oversized inputs and valid extremes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
quietbits
approved these changes
Aug 3, 2026
| bits: number, | ||
| name: string, | ||
| ): void { | ||
| const maxDigits = Math.ceil(bits * 0.30103) + 2; |
Contributor
There was a problem hiding this comment.
Should we put 0.30103 and 2 in a const to make it more clear what it is just by looking at the function?
Ryang-21
added a commit
that referenced
this pull request
Aug 10, 2026
* Class XDR Implementation (#1422) * feat(xdr): codegen tool + schema source * feat(xdr): add class-based XDR runtime and sep51 JSON walker * refactor(numbers): drop LargeInt classes, delegate to new XDR layer * refactor(base): migrate src/base to new XDR layer; drop legacy xdr.ts + generated * refactor: migrate downstream consumers (bindings/contract/horizon/rpc/webauth) * feat(base/scval): add bool to ScValType * allow opaque xdr types to be initalized via string * refactor xdr strings to be represented soley via bytes with a dx friendly XdrString wrapper class * generate a value getter function for void union cases * add a is() function to the generated XDR union classes for instanceOf checks * feat(xdr): regenerate schemas against @stellar/js-xdr and add CAP-71 credentials * feat(xdr): wire the toJSON hook so JSON.stringify emits SEP-0051 * fix(bindings): emit Uint8Array for bytes/bytesN to match scValToNative * refactor(contract): rename fromJSON to fromJson with deprecated aliases * feat(xdr): accept ASCII asset codes with zero padding in constructors * Migrate public API from Buffer to Uint8Array (#1564) * feat(base): migrate crypto and strkey APIs to Uint8Array * feat(base)!: migrate value types to Uint8Array * feat(base)!: migrate transactions, operations, and auth to Uint8Array * feat!: migrate contract, rpc, and webauth layers to Uint8Array * build!: drop buffer polyfill and dependency * fix(horizon): type manage_data value as string to match runtime API * fix(xdr): emit SEP-51 key `type` instead of Rust-escaped `type_` (#1571) * build(xdr): regenerate xdr.json via docker from pinned stellar-xdr commit (#1575) * build(xdr): regenerate schema from stellar-xdr with CAP-83 and CAP-85 ungated (#1576) * build(xdr): regenerate schema from stellar-xdr with CAP-83 and CAP-85 ungated * fix(xdr): keep consumers compiling against the regenerated union arms * fix(vitest): isolate browser dep cache per transport * feat(xdr): support CAP-83 empty tx set values and CAP-85 external executables (#1577) * build(xdr): fail the schema download instead of masking it in a pipe * feat(xdr): support CAP-83 and CAP-85 protocol values * fix(xdr): bound decimal string length before BigInt parse in JSON decode (#1581) * fix(xdr): bound decimal string length before BigInt parse in json decode * refactor(xdr): name the digit-budget constants in bigint-parts * fix(xdr): restrict fromJson to SEP-0051 keys and reject unknown fields (#1582) * fix(xdr): restrict fromJson to SEP-51 keys and reject unknown fields * fix(test): correct horizon corpus fixture path so corpus tests run * fix(strkey): bound decodeCheck input length before base32 decode (#1583) * fix(xdr): reject AssetCode12 JSON codes shorter than 5 bytes (#1585) * fix(horizon)!: make TransactionFailedExtras result_codes.operations optional (#1586) * Fix: signed payload strkey framing (#1588) * fix(strkey): validate signed payload framing in decodeCheck * Fix: xdr json decode validation (#1592) * fix(xdr): throw on unknown union discriminant in fromXdrObject * fix(strkey): validate the claimable balance discriminant byte * fix(xdr): reject non-decimal integer strings in JSON decoding * V17.0.0 rc.1 (#1593) * chore(release): cut v17.0.0-rc.1 * fix(spec): restore instanceof Map check lost in the v17 merge * feat(xdr): add validateXdr static to every generated type (#1597) * feat(xdr): add validateXdr static to every generated type * fix(contract): declare error classes, make types self-contained (#1627) * perf(strkey): reject by length and prefix before decodeCheck throws (#1629) * fix: restore wide-int bounds statics, document Memo.text break (#1628) * fix(xdr): restore wide-int MIN_VALUE/MAX_VALUE statics --------- Co-authored-by: Iveta <[email protected]>
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
SEP-51 JSON decoding of integer fields now checks the decimal string's length before handing it to
BigInt(). A string longer than the target width can possibly hold is rejected up front instead of being parsed and then found out of range.The new
assertDecimalDigitBudget(s, bits, name)insrc/xdr/values/bigint-parts.tsderives the budget from the width: abits-wide integer needs at mostceil(bits * log10(2))decimal digits, plus 2 characters of slack for an optional leading-and a digit of rounding headroom. That gives 22 characters for 64 bits, 41 for 128, and 80 for 256. It is wired into the five decode paths insrc/xdr/values/to-json.ts: theint64/uint64case inwalkFromJson, and thefromJsonoverrides forInt128Parts,Uint128Parts,Int256Parts, andUint256Parts.The check is deliberately scoped to the decode paths, where the SDK is the parser and the string length is chosen by whoever wrote the JSON rather than by the caller. Construction entry points that take a developer-supplied value — the
Int128/Uint128/Int256/Uint256constructors,xdr.Int64/xdr.Uint64,ScInt, andXdrLargeInt— are left as they are: those callers pick the argument, so validating it belongs at their own trust boundary, not inside the SDK.One behavior change worth knowing about: non-canonical spellings that native
BigInt()tolerates — long runs of leading zeros, a leading+, surrounding whitespace, or0x/0o/0bprefixes — are now rejected once they push the string past the budget, so"0".repeat(10000) + "1"throws where it previously decoded as1. SEP-51 integer fields carry canonical decimal strings, so no legitimate input is affected. Every extreme in-range value is covered by a test, including unsigned max and signed min/max for all four widths.No CHANGELOG entry: these decode paths are unreleased, so there is no shipped behavior for this to change.
Why
Parsing an attacker-supplied multi-megabyte decimal string costs work proportional to the input, and produces a value that the range check is then certain to reject. Measured on Node 22, base-10
BigIntparsing runs at roughly 80 ns per input byte — 44 ms for 1M digits, 230 ms for 4M, 1.26 s for 16M. That is linear-ish rather than quadratic, so this is a modest cost rather than a severe one, but it is entirely wasted: the length alone already proves the value cannot fit. Checking it first makes the rejection O(1) and keeps a megabyte-long value out of the interpolated range error message.Memo.fromJson({ id })is the clearest instance — an application decoding a JSON payload from a third party reachesBigInt()on an unbounded string with no opportunity to bound it first.Test plan
test/unit/xdr/xdr_json_bigint_digit_budget.test.tscovers the budget formula per width, the boundary at budget and budget + 1, every decode path against a 1M-digit string, the non-canonical padding case, and the extreme in-range values for all four widths. The regression guard asserts the budget error message and asserts theout of rangemessage is not thrown, which is what distinguishes check-then-parse from parse-then-check; disabling the check fails it. Theto_jsonandbigint_partssuites pass unchanged.