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

Price Manipulation via Reentrancy in Purchase Flow

foxb868

Summary

The purchase() function in Swan.sol allows price manipulation and asset state inconsistencies through reentrancy during token transfers.

Vulnerability Details

https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L289-L300

function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
// Initial checks...
​
// @bug detected: State change before external interactions
listing.status = AssetStatus.Sold;
​
// @bug detected: Multiple external calls creating reentrancy vector
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
// @bug detected: Price manipulation possible during token transfers
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
}

  1. State changes before external calls break the Checks-Effects-Interactions pattern

  2. Multiple token transfers after state changes enable:

    • Reentrancy attacks through malicious tokens

    • Price manipulation during the transaction

    • Asset state inconsistencies

  3. No atomicity guarantees between status updates and transfers

  4. Potential double-spend scenarios if reentered

Attacker could:

  1. Initiate purchase

  2. Status updates to Sold

  3. Reenter through token callbacks

  4. Execute another purchase or manipulate price

  5. Complete original transaction with stale data

This puts both asset transfers and payments at risk of manipulation, potentially leading to loss of funds or assets for the protocol.

Impact

  1. Price manipulation during purchase

  2. Asset state inconsistencies

  3. Potential double spending

  4. Loss of funds for buyers/sellers

Causes:

  1. Violation of CEI pattern

  2. No reentrancy guard despite multiple external calls

  3. Non-atomic state updates and transfers

Tools Used

Manual Review

Recommendations

function purchase(address _asset) external {
+ if (locked) revert ReentrancyGuard();
+ locked = true;
​
AssetListing storage listing = listings[_asset];
require(listing.status == AssetStatus.Listed);
require(listing.buyer == msg.sender);
+ uint256 price = listing.price;
+ address seller = listing.seller;
// Execute transfers first
+ token.transferFrom(msg.sender, address(this), price);
+ SwanAsset(_asset).transferFrom(seller, address(this), 1);
+ SwanAsset(_asset).transferFrom(address(this), msg.sender, 1);
+ token.transfer(seller, price);
​
// Update state last
+ listing.status = AssetStatus.Sold;
+ locked = false;
emit AssetSold(seller, msg.sender, _asset, price);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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