Repository navigation
Validate TxRep integer widths and write uint64 values unsigned - #195
Merged
Merged
Conversation
Hardens the TxRep parser against integers outside their XDR types and fixes the text of large `uint64` values. Before this change, `TxRep::transactionEnvelopeXdrBase64FromTxRep` read every integer with `intval`, a cast or `BigInteger` after stripping one minus sign and checked no width. A `uint32` field outside its range encoded its low 32 bits: `tx.fee: 4294967296` gave a fee of 0 and `tx.fee: -1` a fee of 4294967295. A `uint64` text at or above 2^63 saturated to 9223372036854775807, `tx.seqNum: 9223372036854775808` passed, a negative array length gave an empty array, `0xZZ` read as 0, `--3` as 3, `1e2` as 100, and a `BigInteger` field read `+5` as 0. On the writing side a `uint64` at or above 2^63 was printed as a negative number. A malformed token threw without the field key. This change: - Adds the keyed readers `parseInt32`, `parseUint32`, `parseInt64` and `parseUint64` to `TxRepHelper`, with one token grammar (an optional sign, then decimal digits or `0x` hex digits) and a range check per width. `parseUint64` stores a value at or above 2^63 in the two's-complement form the XDR layer uses. `parseInt` and `parseBigInt` share the grammar and keep their signatures. - Makes the generator choose the reader from the XDR type of every field, array length, typedef, union discriminant and enum ordinal, and print `uint64` fields as unsigned decimals. Regenerates the 87 affected XDR classes and the three generator snapshots. - Replaces a secret seed in the text of these messages with `[redacted seed]`. - Reads the fee, the fee bump fee, the operation and signature counts, the extension version and the memo id of the hand-written envelope parser through the typed readers. The seven hand-written overrides of the time bounds and liquidity pool operations use them too. - Adds tests for malformed tokens, the four widths at their bounds, `uint64` round trips at 2^63 and 2^64-1, enum ordinals, negative and oversized lengths, a leading `+` and the seed redaction. Further tests pin the keyed messages of the facade and the fee bump parser. A generator test pins the reader of five representative fields and that no generated class calls the unkeyed parsers. Every case throws `InvalidArgumentException` with the key and the text; valid TxReps parse as before. The unit suite, PHPStan, the generator tests and the XDR validation pass, and a second regeneration changes no file. A replay of the 41 TxRep texts and 622 integer tokens of the unit suite gives identical bytes and values on both versions. Patch coverage is 232 of 235 changed production lines; the three uncovered lines read `signatures.len` in the generated envelope parsers, which no caller reaches because the facade parses signatures itself. Closes #194
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #195 +/- ##
============================================
+ Coverage 93.41% 93.45% +0.04%
- Complexity 21578 21602 +24
============================================
Files 1006 1006
Lines 52310 52362 +52
============================================
+ Hits 48866 48937 +71
+ Misses 3444 3425 -19
🚀 New features to boost your workflow:
|
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.
Hardens the TxRep parser against integers outside their XDR types and fixes the text of large
uint64values. Before this change,TxRep::transactionEnvelopeXdrBase64FromTxRepread every integer withintval, a cast orBigIntegerafter stripping one minus sign and checked no width. Auint32field outside its range encoded its low 32 bits:tx.fee: 4294967296gave a fee of 0 andtx.fee: -1a fee of 4294967295. Auint64text at or above 2^63 saturated to 9223372036854775807,tx.seqNum: 9223372036854775808passed, a negative array length gave an empty array,0xZZread as 0,--3as 3,1e2as 100, and aBigIntegerfield read+5as 0. On the writing side auint64at or above 2^63 was printed as a negative number. A malformed token threw without the field key.This change:
parseInt32,parseUint32,parseInt64andparseUint64toTxRepHelper, with one token grammar (an optional sign, then decimal digits or0xhex digits) and a range check per width.parseUint64stores a value at or above 2^63 in the two's-complement form the XDR layer uses.parseIntandparseBigIntshare the grammar and keep their signatures.uint64fields as unsigned decimals. Regenerates the 87 affected XDR classes and the three generator snapshots.[redacted seed].uint64round trips at 2^63 and 2^64-1, enum ordinals, negative and oversized lengths, a leading+and the seed redaction. Further tests pin the keyed messages of the facade and the fee bump parser. A generator test pins the reader of five representative fields and that no generated class calls the unkeyed parsers.Every case throws
InvalidArgumentExceptionwith the key and the text; valid TxReps parse as before. The unit suite, PHPStan, the generator tests and the XDR validation pass, and a second regeneration changes no file. A replay of the 41 TxRep texts and 622 integer tokens of the unit suite gives identical bytes and values on both versions. Patch coverage is 232 of 235 changed production lines; the three uncovered lines readsignatures.lenin the generated envelope parsers, which no caller reaches because the facade parses signatures itself.Closes #194