Skip to content

[ANCHOR-1225]: Strict-send sendAmount mis-credited as received USDC lets any wallet user drain a full withdrawal payout for ~$0 on-chain#1967

Merged
amandagonsalves merged 4 commits into
developfrom
fix/anchor-1225
Jul 10, 2026
Merged

[ANCHOR-1225]: Strict-send sendAmount mis-credited as received USDC lets any wallet user drain a full withdrawal payout for ~$0 on-chain#1967
amandagonsalves merged 4 commits into
developfrom
fix/anchor-1225

Conversation

@amandagonsalves

@amandagonsalves amandagonsalves commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Description

For PATH_PAYMENT_STRICT_SEND operations, LedgerClientHelper.convert paired the correct destination asset (destAsset) with the wrong amount (sendAmount - the source's debit, not what the destination received). The real received amount only exists in the operation's execution result (PathPaymentStrictSendResult.success.last.amount), never parsed.

An attacker could send a strict-send payment where sendAmount (in a worthless self-issued asset) met the expected amount while the anchor actually received dust of the real asset - the mismatch guard compared the wrong number and stayed silent, and automated-payout deployments released the full off-chain value. Both observer backends were affected; RPC had no correct value anywhere.

For quoted flows (SEP-31, WITHDRAWAL_EXCHANGE), amount_out is fixed at quote time, and neither the RPC handlers nor the reference server's payout trigger compare amount_in to amount_expected before paying out - they only check that some payment arrived. So the mismatch guard also had to become blocking.

Changes

  • LedgerClientHelper.convert: takes an OperationResult; for PATH_PAYMENT_STRICT_SEND, reads the amount from PathPaymentStrictSendResult.success.last.amount instead of sendAmount, failing closed with LedgerException if the result is missing/non-success/wrong-type. Other operation types unaffected.
  • LedgerClientHelper: added parseOperationResults to extract per-op results from a TransactionResult (fee-bump aware); getLedgerOperations takes the result array and throws on an operations/results length mismatch.
  • Horizon.getTransaction / StellarRpc.fromGetTransactionResponse: both now parse parseResultXdr() and thread the results through, fixing both observer backends.
  • DefaultPaymentListener.processAndDispatchLedgerPayment: added a cross-check (after validate()) comparing the independently-computed PaymentTransferEvent amount/asset against the ledger-parsed values; refuses to dispatch on disagreement.
  • DefaultPaymentListener.checkAndWarnAssetAmountMismatch → renamed checkAssetAmountSufficient, returns boolean. The three handleSepXXTransaction methods skip notifying and all metrics when it's false. Only the amount check blocks - the asset-name check stays advisory, since it's also reached for DEPOSIT-kind rows where it's expected to "mismatch" (off-chain vs on-chain asset); those rows are unreachable here in practice, but there's no need to bet on that.
  • AnchorMetrics: added PAYMENT_OBSERVER_AMOUNT_ASSET_MISMATCH and PAYMENT_OBSERVER_AMOUNT_INSUFFICIENT.
  • LedgerClientHelperTest: updated all convert() call sites for the new signature; added tests for result-amount-not-sendAmount, throw-on-bad-result, parseOperationResults, and the length-mismatch guard.
  • DefaultPaymentListenerTest: added a strict-send fixture with independent event/ledger amount+asset; tests for the happy path, the exploit scenario, and the asset-mismatch case. Fixed createTestTransferEvent() to set sep11Asset so the new gate doesn't regress PAYMENT-path tests. Renamed checkAndWarnAssetAmountMismatch call sites; added tests for the insufficient-amount rejection and its metric.

Acceptance Criteria

  • PATH_PAYMENT_STRICT_SEND credits the result amount, not sendAmount; both observer backends agree.
  • A strict-send payment with inflated sendAmount but dust actual receipt is not credited and does not advance any SEP-6/24/31 transaction.
  • convert throws for a missing/non-success/mismatched-type result rather than defaulting to any amount.
  • DefaultPaymentListener refuses to dispatch (and increments the metric) on event-vs-ledger disagreement, or on insufficient amount.
  • Existing PAYMENT/PATH_PAYMENT_STRICT_RECEIVE handling and test suites are unaffected.

Context

#3810844

Testing

  • Unit: ./gradlew :core:test --tests "org.stellar.anchor.ledger.LedgerClientHelperTest"
  • Unit: ./gradlew :platform:test --tests "org.stellar.anchor.platform.observer.stellar.DefaultPaymentListenerTest"

Documentation

N/A

Known limitations

This PR closes the gap for the observer-driven path (the reported attack surface and default deployment mode); direct/external RPC callers are not addressed here.

* add operation result parsing in stellar and horizon clients

* refactor LedgerClientHelper to extract actual received amount for path_payment_strict_send from operation result

* add a critical security check in DefaultPaymentListener to compare independently computed event amounts and assets with ledger-parsed data

* fix potential amount spoofing vulnerability for path payments
@amandagonsalves amandagonsalves self-assigned this Jul 6, 2026
@amandagonsalves
amandagonsalves marked this pull request as ready for review July 7, 2026 00:21
Copilot AI review requested due to automatic review settings July 7, 2026 00:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a critical accounting flaw for PATH_PAYMENT_STRICT_SEND by ensuring the credited amount reflects what the destination actually received (from the operation result), and adds an observer-side cross-check to refuse processing when independently-computed event data disagrees with ledger-parsed data.

Changes:

  • Parse and thread per-operation results from transaction results (fee-bump aware) so PATH_PAYMENT_STRICT_SEND uses the actual received amount from PathPaymentStrictSendResult.success.last.amount.
  • Add an event-vs-ledger amount/asset cross-check in DefaultPaymentListener and extend unit tests to cover strict-send exploit scenarios.
  • Add a new metric enum entry for observer amount/asset mismatch.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/DefaultPaymentListenerTest.kt Adds strict-send fixtures/tests for observer mismatch blocking and ensures sep11Asset is set in test events.
platform/src/main/java/org/stellar/anchor/platform/service/AnchorMetrics.java Adds a metric constant for observer amount/asset mismatches.
platform/src/main/java/org/stellar/anchor/platform/observer/stellar/DefaultPaymentListener.java Adds event-vs-ledger amount/asset cross-check before dispatching a received payment.
core/src/test/kotlin/org/stellar/anchor/ledger/LedgerClientHelperTest.kt Updates convert signature usage and adds tests for strict-send result amount parsing and operation-result parsing behavior.
core/src/main/java/org/stellar/anchor/ledger/StellarRpc.java Parses resultXdr and threads operation results into getLedgerOperations.
core/src/main/java/org/stellar/anchor/ledger/LedgerClientHelper.java Adds strict-send received-amount extraction from op results, adds parseOperationResults, and enforces ops/results length matching.
core/src/main/java/org/stellar/anchor/ledger/Horizon.java Parses resultXdr and threads operation results into getLedgerOperations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/src/main/java/org/stellar/anchor/ledger/LedgerClientHelper.java Outdated
* refactor payment amount validation to reject payments smaller than expected

* add new metric for insufficient payment amounts

* update payment processing to halt when amount is insufficient

* add tests for payment rejection and metric incrementation
* refactor `extractStrictSendReceivedAmount` to clarify path payment strict send result parsing
* add explicit null checks for nested `pathpaymentstrictsendresult` objects
* add tests to validate `ledgerexception` when critical result components are missing
@amandagonsalves
amandagonsalves requested a review from JiahuiWho July 7, 2026 12:15
@amandagonsalves
amandagonsalves merged commit 546503b into develop Jul 10, 2026
17 of 21 checks passed
@amandagonsalves
amandagonsalves deleted the fix/anchor-1225 branch July 10, 2026 17:00
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.

3 participants