Fix: signed payload strkey framing - #1588
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Strengthens signed-payload StrKey decoding by enforcing SEP-23 framing consistently across raw, validation, and JSON decoding paths.
Changes:
- Validates payload length, total framed size, and zero padding.
- Adds malformed-input and boundary round-trip tests.
- Updates changelog and generated API references.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/base/strkey.ts |
Adds signed-payload framing validation. |
test/unit/base/strkey.test.ts |
Covers malformed and valid boundary cases. |
docs/reference/core-keys.md |
Regenerates source links. |
CHANGELOG.md |
Documents the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ryang-21
changed the title
Ryang-21
marked this pull request as ready for review
August 4, 2026 18:49
quietbits
approved these changes
Aug 4, 2026
7 tasks
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
decodeChecknow validates the internal framing of a signed-payload (P...) strkey, soStrKey.decodeSignedPayload,StrKey.isValidSignedPayload, and the SEP-0051 JSON decoder all reject a malformed one.SEP-23 step 4 defines the body of a
P...strkey as 32 bytes of ed25519 key, a 4-byte payload length in network byte order, the payload, then 0-3 zero bytes of padding so the payload plus padding is a multiple of four. Previously the SDK checked the encoded string length, base32 canonicality, the version byte, and the checksum — but trusted the length word inside the body.decodeChecknow also requires that the declared length is 1-64, that the decoded body is exactly the size that length implies, and that the padding bytes are zero.Since the JSON override for
SignerKeycallsStrKey.decodeSignedPayload, andisValidalready wraps itsdecodeCheckcall in atry/catch, one check covers all three entry points. The decoded-size range check inisValidis now redundant and was removed.Why
SEP-23 lists three invalid
P...test cases that implementations must reject. The SDK accepted all three:declared length 32 needs 68 bytes, got 72declared length 29 needs 68 bytes, got 64declared length 29 needs 68 bytes, got 65All eight SEP-23 valid cases still decode, and the other twelve invalid cases were already rejected.
Accepting these meant the two decoders in the SDK disagreed about the same value. A declared length larger than the bytes actually present read out as a truncated payload, because
subarrayclamps an out-of-range end instead of throwing; bytes past the declared length were never read at all. So the JSON path returned a value whereSignerKey.fromXdrthrew, and re-encoding it produced a different signer key than the input described, with no error either way.StrKey.isValidSignedPayloadreturnedtrue, so a caller validating before decoding got no signal.The 1-64 lower bound matches rs-stellar-strkey, the reference implementation SEP-23 points to, which documents the inner payload as
1..=64because stellar-core'sSetOptionsOpFramerejects an empty one withSET_OPTIONS_BAD_SIGNER. It also matches the SDK's existing behavior, which already reported an empty payload as invalid.