Summary
The Swan protocol's purchase mechanism contains critical race conditions where asset status updates occur before transfers complete, allowing market manipulation and potential asset/payment desynchronization. This can lead to assets being marked as sold while remaining with the seller, or payments being processed without asset delivery.
The purchase flow in Swan.sol has multiple synchronization issues: 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];
if (listing.status != AssetStatus.Listed) {
revert InvalidStatus(listing.status, AssetStatus.Listed);
}
listing.status = AssetStatus.Sold;
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
}
Vulnerability Details
The issue is the status is updated to Sold before the actual transfers occur, if either transfer fails, the asset remains with the seller but the status is incorrectly marked as Sold. This creates an inconsistent state where an asset is marked sold but ownership hasn't changed
function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
if (listing.status != AssetStatus.Listed) {
revert InvalidStatus(listing.status, AssetStatus.Listed);
}
if (listing.buyer != msg.sender) {
revert Unauthorized(msg.sender);
}
listing.status = AssetStatus.Sold;
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
}
This is problem because assets can be purchased in expired rounds, breaking the round-based market design. Purchases can occur outside designated purchase phases, disrupting market timing controls and status updates before transfers create inconsistent states if transfers fail.
POC
await swan.purchase(assetAddress);
Impact
Assets can become permanently locked in "Sold" state without ownership transfer
Payments may process without asset delivery
Market round integrity compromised through out-of-phase purchases
Potential for double-spending through failed transfer recovery attempts
Recommendations
function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
+ // Validate round and phase
+ (uint256 currentRound, BuyerAgent.Phase phase,) = BuyerAgent(listing.buyer).getRoundPhase();
+ require(currentRound == listing.round && phase == BuyerAgent.Phase.Buy, "Invalid purchase timing");
+ // Check approvals and balances
+ require(SwanAsset(_asset).isApprovedForAll(listing.seller, address(this)), "Asset not approved");
+ require(token.allowance(msg.sender, address(this)) >= listing.price, "Insufficient allowance");
// Execute transfers before status update
+ bool success = true;
+ success &= SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
+ success &= SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
+ success &= token.transferFrom(listing.buyer, address(this), listing.price);
+ success &= token.transfer(listing.seller, listing.price);
+ require(success, "Transfer failed");
// Update status only after successful transfers
listing.status = AssetStatus.Sold;
}
