Skip to content

rpc: reject trailing bytes in sendrawtransaction#2556

Closed
Lrifton92 wants to merge 1 commit into
btcsuite:masterfrom
Lrifton92:fix/sendrawtransaction-trailing-bytes
Closed

rpc: reject trailing bytes in sendrawtransaction#2556
Lrifton92 wants to merge 1 commit into
btcsuite:masterfrom
Lrifton92:fix/sendrawtransaction-trailing-bytes

Conversation

@Lrifton92

Copy link
Copy Markdown

Problem

handleSendRawTransaction deserializes the supplied hex into a wire.MsgTx but never verifies that the entire input was consumed:

var msgTx wire.MsgTx
err = msgTx.Deserialize(bytes.NewReader(serializedTx))

MsgTx.Deserialize reads exactly the bytes that make up a transaction and stops, so any extra bytes appended after a fully-formed transaction are silently discarded — and the transaction is still accepted and relayed.

This diverges from bitcoind, whose sendrawtransaction rejects such input with a deserialization error (TX decode failed) instead of broadcasting a transaction parsed from a malformed payload.

Fix

Read the serialized transaction through a *bytes.Reader and, after a successful Deserialize, check that no unread bytes remain. If any are left over, return ErrRPCDeserialization, matching Core's behavior.

This mirrors the same trailing-bytes hardening applied to decoderawtransaction in #2550 and to the PSBT global unsigned tx in #2541 — completing the set across the RPC tx-decode paths.

Test

TestHandleSendRawTransactionTrailingBytes appends a spurious byte to an otherwise valid transaction and asserts the handler now returns ErrRPCDeserialization. Verified red before the fix (the handler proceeded past deserialization) and green after.

go build ./..., go vet ., gofmt -l and the RPC handler tests all pass.

handleSendRawTransaction deserialized the supplied hex into a MsgTx but
never verified that the entire input was consumed. wire.MsgTx's
Deserialize reads exactly the bytes that make up a transaction from the
reader and stops, so any extra bytes appended after a fully-formed
transaction were silently discarded and the transaction was still
accepted and relayed.

This diverges from bitcoind, which rejects such input with a
deserialization error ("TX decode failed") rather than broadcasting a
transaction parsed from a malformed payload.

Read the serialized transaction through a *bytes.Reader and, after a
successful Deserialize, check that no unread bytes remain. If any are
left over the input is not a valid serialized transaction and an
ErrRPCDeserialization error is returned, matching Core's behavior. This
mirrors the same hardening applied to decoderawtransaction in btcsuite#2550 and
to the PSBT global unsigned tx in btcsuite#2541.

A regression test exercises the trailing-bytes case by appending a
spurious byte to an otherwise valid transaction and asserting the
handler now returns ErrRPCDeserialization.

Signed-off-by: Lrifton92 <[email protected]>

@allocz allocz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@starius

starius commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Replacing this PR with #2558

@Roasbeef

Roasbeef commented Jul 1, 2026

Copy link
Copy Markdown
Member

Folded into #2558

@Roasbeef Roasbeef closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants