strkey: validate payload length in DecodeSignedPayload to prevent panic#5909
Conversation
strkey.Decode() accepts payloads of any length with valid checksums. Without length validation, short P-addresses could pass DecodeSignedPayload() and later cause panics when slicing for the 32-byte signer key. Add explicit length check before slicing to reject malformed signed payloads early with a descriptive error.
There was a problem hiding this comment.
Pull request overview
This pull request fixes a panic vulnerability in strkey.DecodeSignedPayload() when processing malformed P-addresses with payloads shorter than 32 bytes. The function previously attempted to slice the first 32 bytes for the signer key without validating length, causing a runtime panic on short inputs.
Changes:
- Added length validation in
DecodeSignedPayload()to reject payloads shorter than 32 bytes before slicing - Added comprehensive test coverage for edge cases (0, 10, and 31 byte payloads) to verify rejection with appropriate error messages
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| strkey/signed_payload.go | Added length check before slicing to prevent panic when raw payload is shorter than 32 bytes |
| strkey/signed_payload_test.go | Added test to verify short raw payloads are rejected with descriptive error message |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if len(raw) < signerLen { | ||
| return nil, errors.Errorf("signed payload too short: %d bytes", len(raw)) | ||
| } |
There was a problem hiding this comment.
The length in the core and rust impls actually require an inner payload length of 1.
But that seems out of scope to fix in this pr since this Pr is just fixing the unsafe slicing causing the panic. But fyi none the less and maybe we should make the Go impl more similar to the others in a future change.
strkey.Decode() accepts payloads of any length with valid checksums. Without length validation, short P-addresses could pass DecodeSignedPayload() and later cause panics when slicing for the 32-byte signer key.
Add explicit length check before slicing to reject malformed signed payloads early with a descriptive error.
This fixes issue #5902