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

Inconsistent states due to lack of atomicity

kwakudr

Summary

Inconsistent states when transactions fail since the state is already updated before the transaction failure. The transaction will not revert in some situations.

Vulnerability Details

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);
}
​
​
// update asset status to be sold
listing.status = AssetStatus.Sold;
​
​
// transfer asset from seller to Swan, and then from Swan to buyer
// this ensure that only approval to Swan is enough for the sellers
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
​
​
// transfer money
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
​
​
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}

The contract only has three states, which doesn't account for:

Transaction processing states

  • Error states

  • Dispute resolution

  • Temporary holds

  • Cancellations

Current possible transitions are limited to:

[Non-existent] -> Listed -> Sold

This creates several problems:

  • No way to pause a listing

  • No way to handle failed transactions

  • No way to dispute a sale

  • No way to cancel a listing

  • No way to handle partial payments

Status is updated before transfers complete. There is No way to revert status if transfers fail. This could leave asset in inconsistent state. When the transfer fails the buyer will receive the asset but the seller will not receive their money which is a loss for the seller.

Impact

Inconsistent states due to lack of atomicity

Tools Used

Manual Review

Recommendations

Create atomic operations so that when transfers or transactions fail, status would revert. Also it would be good to Enhance the Status Enum with other states as listed above.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[KNOWN] - Low-35 Unsafe use of transfer()/transferFrom() with IERC20

Support

FAQs

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