fix(sdk-coin-stx): verify recipient and amount in native STX verifyTransaction#9244
Merged
Merged
Conversation
Contributor
bitgo-ai-agent-dev
Bot
force-pushed
the
cshld-839-stx-verify-transaction
branch
from
July 14, 2026 07:01
eac74de to
cce41e6
Compare
prajwalu142
force-pushed
the
cshld-839-stx-verify-transaction
branch
3 times, most recently
from
July 14, 2026 16:34
6d7a890 to
fcbb68f
Compare
Contributor
|
@claude review this pr for potential regressions and provide verdict |
Contributor
|
Claude finished @prajwalu142's task —— View job PR Review for STX verifyTransaction Security FixTasks:
Review SummaryThis PR addresses a critical security vulnerability in the STX ✅ VERDICT: APPROVEThis PR should be merged - it fixes a high-severity security issue without introducing regressions. Detailed AnalysisSecurity Fix QualityExcellent - The implementation properly validates:
Implementation AssessmentCode Quality: Good
Dependencies: Safe
Test CoverageComprehensive - 8 new tests cover:
Regression AnalysisNo Breaking Changes Detected:
Performance ImpactMinimal - One additional Minor Observations
This security fix is essential and ready for production deployment. |
Ranjna-G
reviewed
Jul 15, 2026
Ranjna-G
reviewed
Jul 15, 2026
bitgo-ai-agent-dev
Bot
force-pushed
the
cshld-839-stx-verify-transaction
branch
5 times, most recently
from
July 15, 2026 07:19
a575d5d to
9dff461
Compare
…ansaction Previously, Stx.verifyTransaction was a near no-op: it only checked that the recipient count was <= 1. A compromised prebuild could redirect native STX sends to any address or alter the amount, and local verification would still pass. Port the decode-and-compare pattern from Sip10Token.verifyTransaction into Stx.verifyTransaction, adapted for single-recipient STX. The new implementation decodes the prebuild txHex via explainTransaction and directly compares the single decoded output's address and amount against txParams.recipients[0], then validates the memo. No iteration needed since STX enforces a single-recipient constraint at the top of the method. This closes the gap where the SIP10 token path already had full recipient validation but native STX did not. Ticket: CSHLD-839 Session-Id: 7de30368-fc1b-4b09-9d2e-55be78e462dc Task-Id: 2203e736-35f2-4e2f-8cde-6cdbd0d3e2b6
prajwalu142
force-pushed
the
cshld-839-stx-verify-transaction
branch
from
July 15, 2026 09:34
9dff461 to
aea2d73
Compare
prajwalu142
marked this pull request as ready for review
July 16, 2026 05:11
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
Stx.verifyTransactionpreviously returnedtrueafter checking only that the recipient count was ≤ 1 — no address or amount comparison was performed.Sip10Token.verifyTransaction:txHexviaexplainTransactiontxParams.recipientstxParams.memoor embedded in recipient address) against the decoded transaction memogetMemoIdAndBaseAddressFromAddressimport from existing utilslodashimport (already a listed dependency) for_.isEqualWhy
verifyTransactionwould still returntruewithout detecting the mismatch.Test plan
cd modules/sdk-coin-stx && mocha)truetxHexin prebuild throws an errorsdk-coin-stxwith no new errorsTicket: CSHLD-839