fix: prevent dust attack order hijacking and set order_percent in early update#697
Open
onahprosper wants to merge 1 commit intostablefrom
Open
fix: prevent dust attack order hijacking and set order_percent in early update#697onahprosper wants to merge 1 commit intostablefrom
onahprosper wants to merge 1 commit intostablefrom
Conversation
- Introduced a new test file `indexer_test.go` to validate the behavior of the `UpdateReceiveAddressStatus` function. - Implemented tests to ensure correct handling of various token transfer scenarios, including dust transfers, valid transfers, and ignored transfers based on address and duplicate transaction hashes. - Enhanced the `UpdateReceiveAddressStatus` function to skip dust transfers, preventing address poisoning attacks. - Updated the `ProcessPaymentOrderFromBlockchain` function to set the order percent to 100 when processing payment orders.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Description
This PR addresses two deposit processing bugs:
1. Dust/address poisoning attack vulnerability
When a deposit is detected on a
receiveAddress,UpdateReceiveAddressStatusaccepts any transfer amount and mutates the order to match — including dust transfers as small as $0.005. An attacker can send a dust transfer to areceiveAddressbefore the real deposit arrives. The system processes the dust first, rewrites the order amount from (e.g.) $5 to $0.005, transitions the order through deposited → refunding → refunded, and the real $5 deposit is left stranded.Fix: Reject transfers with
event.Value <= 0.1(token units) before any order mutation occurs. The order stays ininitiatedstatus so the real deposit is processed normally when it arrives. This works across all chains (EVM, Tron, Starknet) since the check operates on the already-normalized token-unit value.2. Orders stuck in
settlingwithorder_percent = 0In
ProcessPaymentOrderFromBlockchain, when an existing order (found bymessageHash) is updated, the early Phase 1 update (lines 97-105) setsGatewayIDand status but does not setorder_percent. IfvalidateAndPreparePaymentOrderDatasubsequently fails or cancels the order, the function returns early. The order now hasGatewayIDset (so the indexer skips it permanently on retry) butorder_percentremains at the schema default of 0. When settlement is attempted, it sends 0% to the on-chain contract, which reverts, and the order is stuck in asettling↔validatedretry loop.Fix: Include
SetOrderPercent(decimal.NewFromInt(100))in the early Phase 1 update so it's always set alongsideGatewayID, regardless of whether validation succeeds.References
N/A
Testing
Dust threshold — 5 new unit tests in
services/common/indexer_test.go:TestUpdateReceiveAddressStatus_DustTransferIgnored— 0.005 transfer is rejected, order stays initiatedTestUpdateReceiveAddressStatus_BoundaryDustIgnored— exactly 0.1 transfer is rejected (boundary)TestUpdateReceiveAddressStatus_ValidTransferProcessed— 5.1 transfer passes the check and moves the order forwardTestUpdateReceiveAddressStatus_WrongAddressIgnored— transfer to a different address has no effectTestUpdateReceiveAddressStatus_DuplicateTxHashIgnored— already-indexed txHash is skippedRun:
go test ./services/common/ -vorder_percent fix — verified by code inspection. For orders currently stuck in
settlingwithorder_percent = 0, a one-time DB update to setorder_percent = 100will unblock them via the existing stale_ops retry.Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.