Skip to content

fix crash in Descriptor::parse_desc found by fuzzer #809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 28, 2025

Conversation

apoelstra
Copy link
Member

@apoelstra apoelstra commented Apr 21, 2025

Thanks to Bruno Garcia and i-am-yuvi who independently found this crash and reported it to me.

Fixes #806

Needs backport.

@apoelstra
Copy link
Member Author

cc @i-am-yuvi @brunoerg

When parsing a descriptor with `Descriptor::parse_descriptor`, we first parse
as a string and then parse the keys. We fail to consider parsing errors
in the keys, resulting in a panic.

Also, clean up the panic message so it's clearer what's going on.
@apoelstra
Copy link
Member Author

Bug originates in #493

Copy link
Member Author

@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.

On 6bff186 successfully ran local tests

@brunoerg
Copy link
Contributor

code review ACK 6bff186; haven't tested

Copy link

@i-am-yuvi i-am-yuvi left a comment

Choose a reason for hiding this comment

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

Tested and Code Review ACK 6bff186

cargo test regression_806


Compiling miniscript v13.0.0 (...rust-miniscript)
Finished `test` profile [unoptimized + debuginfo] target(s) in 3.99s
Running unittests src/lib.rs (target/debug/deps/miniscript-0203e4fe382223fb)

running 1 test
test descriptor::tests::regression_806 ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 119 filtered out; finished in 0.00s

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 6bff186

Thanks for the reports, Bruno and i-am-yuvi

@apoelstra apoelstra merged commit 6a3f0f7 into rust-bitcoin:master Apr 28, 2025
31 checks passed
@apoelstra apoelstra deleted the 2025-04--fuzz-crash branch April 28, 2025 21:52
apoelstra added a commit that referenced this pull request Apr 29, 2025
212d78a bump patch version of 12.3 (Andrew Poelstra)
4c6366a add regression test for #806 (Andrew Poelstra)
4823d86 descriptor: fix key parsing error handling in parse_desc (Andrew Poelstra)

Pull request description:

  Backports #809 and does another patch release.

ACKs for top commit:
  sanket1729:
    reACK 212d78a

Tree-SHA512: c95651f29e1bcb4e8c8036b2a74443c3fd818d39c259852d5c35f0f5ef88fbbe18b312eb7e4d29c39a4af8e9660871fbe998d86c9d21a3d502f1d9854f6f7a43
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.

fuzz: panic in Descriptor::parse_descriptor with Invalid pkh() Input in rust_miniscript
4 participants