fix(sdk-coin-xtz): verify destination and amount in verifyTransaction#9245
Draft
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
Draft
fix(sdk-coin-xtz): verify destination and amount in verifyTransaction#9245bitgo-ai-agent-dev[bot] wants to merge 1 commit into
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
Conversation
Previously, Xtz.verifyTransaction only checked that the recipient count was at most 1, then returned true unconditionally. This meant a compromised prebuild service could redirect an XTZ transfer to an attacker-controlled address and the local signer would still sign it. This fix decodes the transaction from txPrebuild.txHex when a single recipient is present and compares the encoded destination address and amount against txParams.recipients[0]. Any decode failure or mismatch throws, failing closed to prevent silent fund redirection. Behavior is unchanged when no recipients or no txHex are present (e.g. wallet initialization transactions), preserving backward compatibility. Ticket: CSHLD-838 Co-Authored-By: Claude <noreply@anthropic.com> Session-Id: 4815208a-ec3a-411f-a659-358c0aaa25e6 Task-Id: 0ce30aac-a75b-4b67-acca-38533b9d28f0
Contributor
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
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.
What
Xtz.verifyTransactionnow decodestxPrebuild.txHexand compares the encoded destination address and transfer amount againsttxParams.recipients[0]txHexare present (wallet initialisation)Why
verifyTransactionpreviously only checked that the recipient count was ≤ 1 and returnedtrueunconditionallytxParams.recipients) matches the bytes that will actually be signed and broadcastTest plan
should verify a valid transaction where destination and amount match— passes for a matching prebuildshould reject when prebuild destination does not match requested recipient— catches address redirectionshould reject when prebuild amount does not match requested recipient— catches amount manipulationshould reject when the prebuild contains an unexpected number of outputs— catches unexpected multi-output buildsshould reject when the prebuild cannot be decoded— fail-closed on corrupt/invalid txHexshould pass when no recipients are specified— backward-compatible for wallet-init flowsshould pass when no txHex is present in the prebuild— backward-compatible for wallet-init flowsyarn run unit-test --scope @bitgo/sdk-coin-xtz)Ticket: CSHLD-838