Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Non-Atomic Asset Transfer Pattern Leads to Asset Lock and Payment Desynchronization

aariiif

Summary

The purchase function in Swan.sol implements a non-atomic two-step transfer pattern where assets flow through the protocol before reaching the buyer. Combined with premature status updates and delayed payment processing, this creates scenarios where assets can be locked, payments can fail after transfers, and the protocol temporarily takes custody of assets against the P2P trading principle.

Vulnerability Details

The core issue lies in the sequence of operations in the purchase function: https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/Swan.sol#L276-L299

function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
​
// asset must be listed to be purchased
if (listing.status != AssetStatus.Listed) {
revert InvalidStatus(listing.status, AssetStatus.Listed);
}
​
// can only the buyer can purchase the asset
if (listing.buyer != msg.sender) {
revert Unauthorized(msg.sender);
}
​
// @Issue - Status is updated before transfers are complete
// This creates a state where the asset is marked as sold but transfers could still fail
listing.status = AssetStatus.Sold;
​
// @Issue - Non-atomic transfer pattern creates temporary custody
// The protocol temporarily takes custody of the asset, violating direct P2P transfer principle
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
​
// @Issue - Payment happens after asset transfers
// Token transfers should happen before asset transfers to prevent payment after failed transfers
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
​
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}

The transfers should be atomic, but the current implementation uses a two-step transfer process where the asset first moves to Swan contract then to the buyer.

  1. Why It's Happening:

  • The contract implements a double transfer pattern to avoid requiring multiple approvals

  • However, this creates a state where the asset is temporarily held by the Swan contract

  • The specification rule fails because there's a moment where the transfer is "active" but not complete

For the following reasons,

  1. Premature Status Update:

  • The listing is marked as sold before ensuring the transfers succeed

  • If transfers fail, the listing remains in "Sold" state but ownership hasn't changed

  • This could permanently lock assets in an invalid state

  1. Non-atomic Transfer Pattern:

  • Creates a temporary state where the protocol holds the asset

  • Violates the direct P2P trading principle

  • Introduces unnecessary custody risk

  • Creates potential for front-running between transfers

  1. Payment Sequencing:

  • Payment processing after asset transfer creates risk

  • If payment fails, asset ownership has already changed

  • No mechanism to revert the asset transfer if payment fails

Impact

  • Assets can be permanently locked in "Sold" state if transfers fail

  • Protocol temporarily holds assets, violating P2P trading principles

  • Failed payments after successful transfers create unrecoverable states

Recommendations

function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
+ // Process payment first
+ token.transferFrom(listing.buyer, listing.seller, listing.price);
+ // Direct P2P transfer
+ SwanAsset(_asset).transferFrom(listing.seller, listing.buyer, 1);
+ // Update status after successful transfers
+ listing.status = AssetStatus.Sold;
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.