Fix stack/heap buffer overflows in Zcash and Bitcoin builders, and prevent TWAnyAddressData crash on mismatched address type - #4801
Open
nikhil-gupta-tw wants to merge 6 commits into
Open
Conversation
… buffer overflow from attacker-controlled TransactionPlan
… heap buffer overflow in UTXO transaction builder
…terminate when legacy Bitcoin address is used as BitcoinCash
Binary size comparison➡️ aarch64-apple-ios: 14.31 MB ➡️ aarch64-apple-ios-sim: 14.31 MB ➡️ aarch64-linux-android: 18.73 MB ➡️ armv7-linux-androideabi: 16.17 MB ➡️ wasm32-unknown-emscripten: 13.66 MB |
BSCSecChef
reviewed
Jun 17, 2026
BSCSecChef
left a comment
BSCSecChef
left a comment
Collaborator
There was a problem hiding this comment.
Why not use NU6BranchID here as default? -
sergei-boiko-trustwallet
approved these changes
Jul 14, 2026
sergei-boiko-trustwallet
left a comment
sergei-boiko-trustwallet
left a comment
Contributor
There was a problem hiding this comment.
LGTM
Just a few questions
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
3 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.
This pull request introduces important bug fixes and validation improvements across several transaction builder modules, as well as enhanced error handling in the address interface. The main changes ensure safer index handling for output insertion, stricter validation of transaction parameters, and improved robustness against exceptions.
Transaction output insertion safety:
Bitcoin/TransactionBuilder.handZen/TransactionBuilder.hto clamp theoutputOpReturnIndexto the valid range, preventing out-of-bounds errors when inserting OP_RETURN outputs. [1] [2]Parameter validation:
Zcash/TransactionBuilder.hto verify that the providedbranchIdmatches the expected size, returning an error if it does not, thus preventing invalid transaction construction.Error handling:
TWAnyAddressDatafunction ininterface/TWAnyAddress.cppwith a try-catch block to return an empty data object if an exception occurs, improving stability and preventing crashes.