Fix index out of bounds bug#130
Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom Sep 19, 2023
Merged
Conversation
Currently we index into the data field without first checking it is not empty. For context, the indexing is done _before_ we do segwit validity checks which check for correct data lengths. This is a real noob bug , bad Tobin - no biscuit. Add a check for the empty array before indexing into it.
Add a unit test that checks various invalid segwit addresses.
Member
|
Nice, win for fuzzing. I guess I should've caught this in review -- though it's tough with big PRs like this, especially with lots of iteration. |
apoelstra
approved these changes
Sep 19, 2023
Member
|
Will go ahead and merge this without @clarkmoody since it's small and purely a bugfix. |
apoelstra
added a commit
that referenced
this pull request
Sep 29, 2023
79918f6 Re-write crate level API (Tobin C. Harding) Pull request description: Now includes updates to fuzzing and is on top of #130 (bug fix). Now we have all the new `primitives` pieces in place we can re-write the public API to take advantage of them - WIN! Close: #126 ACKs for top commit: apoelstra: ACK 79918f6 clarkmoody: ACK 79918f6 Tree-SHA512: 00b575e9bbd0f3768cbea561a6f72425ff4052e88d624e99f1773f61835d104f52c841fa21f9e5cda1347f83e1d8ada75d9c314b9e16fef9f68091fbf1f53b6a
apoelstra
added a commit
that referenced
this pull request
Oct 9, 2023
8279efa Bump version to 0.10.0-beta (Tobin C. Harding) Pull request description: In preparation for doing the next dev release add a changelog entry and bump the version number to `0.10.0-beta`. We need to release the bug fix in #130 because fuzzing is hitting it in `rust-bitcoin`. This release would be better to use in `rust-miniscript` as well gearing up for the real 0.10.0 release. ACKs for top commit: clarkmoody: ACK 8279efa apoelstra: ACK 8279efa Tree-SHA512: d2fdac35717b06bd79a5ae18e897fcaf39a570b2998b8f51ab86e46f4405b7f77284c164ff4ad19c80aafc26e8433526cd20276ef485cbbf573d032037d7a49b
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
2 participants
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.
Noob bug here, I accessed an array without first checking that it was non-empty :(
Adds unit test as a separate patch, re-arrange to verify the bug.
Discovered while fuzzing #128.