Skip to content

psbt: Fix bug in Subtype consensus_encode#3519

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:10-28-fix-fuzzing
Oct 29, 2024
Merged

psbt: Fix bug in Subtype consensus_encode#3519
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:10-28-fix-fuzzing

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Oct 27, 2024

In #2906 we switched from using a u8 for type keys to using a u64 and encoding as a compact int (inline with the spec). Note that a u8 encodes to the same bytes as a u64 when the value is < 252.

In that patch, I introduced a bug because the length returned by PoprietaryKey::consensus_encode uses a hard code 1 for the length of the encoding (because of single byte) instead of the variable length for the new compact encoding.

Bug showed up in fuzzing, and was isolated by Jamil - mad props.

Fix: #3501

In rust-bitcoin#2906 we switched from using a `u8` for type keys to using a `u64`
and encoding as a compact int (inline with the spec). Note that a `u8`
encodes to the same bytes as a `u64` when the value is < 252.

In that patch, I introduced a bug because the length returned by
`PoprietaryKey::consensus_encode` uses a hard code 1 for the length of
the encoding (because of single byte) instead of the variable length for
the new compact encoding.

Bug showed up in fuzzing, and was isolated by Jamil - mad props.
@github-actions github-actions bot added the C-bitcoin PRs modifying the bitcoin crate label Oct 27, 2024
@tcharding
Copy link
Copy Markdown
Member Author

cc @jamillambert

Copy link
Copy Markdown
Contributor

@jamillambert jamillambert left a comment

Choose a reason for hiding this comment

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

Fixes the bug and I get no Fuzz errors locally.

ACK c89b816

@tcharding
Copy link
Copy Markdown
Member Author

BOOM!

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK c89b816; successfully ran local tests

@apoelstra apoelstra merged commit 6c8d0ef into rust-bitcoin:master Oct 29, 2024
@tcharding tcharding deleted the 10-28-fix-fuzzing branch November 1, 2024 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bitcoin PRs modifying the bitcoin crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fuzz failure in bitcoin_deserialize_psbt

3 participants