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

Double Payment Vulnerability in Swan Purchase Flow

foxb868

Summary

Critical value conservation violation in Swan's purchase mechanism where sellers receive excess funds due to unaccounted royalty fees.

Vulnerability Details

Looking at the purchase() function in Swan.sol, there is a value leakage issue: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L276-L302

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);
​
// @bug detected: Double payment - seller receives full price despite already collecting royalties
// Royalty fees were already collected in transferRoyalties() during listing,
// but seller still receives the full listing price here
token.transferFrom(listing.buyer, address(this), listing.price);
token.transfer(listing.seller, listing.price);
​
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}

The bug is when transferring funds:

  1. The full listing price is transferred from the buyer to the Swan contract

  2. The same full amount is transferred to the seller

  3. However, the royalty fees that were collected earlier in transferRoyalties() are not accounted for

This means the seller receives the full price even though royalty fees were already deducted, creating a value conservation violation where:

buyerValueAfter + sellerValueAfter > buyerValueBefore + sellerValueBefore

This vulnerability creates a value conservation violation in the protocol's economic model:

  1. During listing, sellers pay royalty fees through transferRoyalties()

  2. During purchase, they receive the full listing price without fee deduction

  3. This leads to overpayment where sellers effectively get refunded their royalty fees

The impact is significant because:

  • It breaks the protocol's fee collection mechanism

  • Creates unfair economic advantage for sellers

  • Violates the core value conservation invariant proven by Certora

  • Scales with listing price and number of transactions

PoC

// Initial balances
seller = 100 ETH
buyer = 100 ETH
​
// Listing with 10% royalty fee
price = 10 ETH
royaltyFee = 10%
​
// During listing (transferRoyalties)
seller pays 1 ETH royalty (10% of price)
seller balance = 99 ETH
​
// During purchase
buyer pays 10 ETH
seller receives 10 ETH
seller final balance = 109 ETH // Should be 108 ETH
​
// Net result
Seller profits extra 1 ETH from royalty refund
Protocol loses 1 ETH in fees

This vulnerability is particularly dangerous because:

  1. It systematically drains protocol fees

  2. Scales proportionally with listing prices

  3. Affects every purchase transaction

  4. Creates unfair economic advantage for sellers

  5. Violates core protocol invariants

The issue connects directly to Swan's core marketplace functions and fee collection mechanism, making it a critical vulnerability requiring immediate attention.

Impact

The contract fails to account for previously collected royalty fees during the purchase flow. This connects to two key functions:

  1. transferRoyalties() - Collects fees during listing

  2. purchase() - Transfers full amount without fee adjustment

  • Sellers receive more than intended (listing price + refunded royalties)

  • Protocol loses fee revenue

Tools Used

Value Conservation Invariant Testing

Recommendations

function purchase(address _asset) external {
AssetListing storage listing = listings[_asset];
- token.transferFrom(listing.buyer, address(this), listing.price);
- token.transfer(listing.seller, listing.price);
+ uint256 buyerFee = (listing.price * listing.royaltyFee) / 100;
+ token.transferFrom(listing.buyer, address(this), listing.price);
+ token.transfer(listing.seller, listing.price - buyerFee);
}

Alternatively:

  • Implement royalty tracking per listing

  • Add fee accounting system

  • Consider implementing escrow mechanism for fees

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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