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

A malicious buyer can collect the `royaltyFee` from `Swan::transferRoyalties` without making any purchases by setting the `amountPerRound` variable very low.

saurabh_singh

Summary

A malicious buyer can set their amountPerRound variable very low, leading to a BuyLimitExceeded revert in the BuyerAgent::purchase function. This would prevent them from making any purchases while still collecting the royalty fees from sellers who list and relist their assets.

Vulnerability Details

A malicious buyer can collect the royaltyFee without making any purchases by setting the amountPerRound variable very low in the BuyerAgent::purchase function. This will trigger a BuyLimitExceeded revert, preventing the attacker from spending any money while still gaining the royalty fee each time an asset is listed or relisted.

This is beacuse the transferRoyaltiesfunction is called in the Swan::listand Swan::relistfunction instead of Swan::purchase.

The automatic purchase will also revert be setting the amountPerRound low.

The revert BuyLimitExceeded-> https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L244

transferRoyalties call in Swan::list function-> https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/Swan.sol#L188

transferRoyalties call in Swan::relist function-> https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/Swan.sol#L252

function transferRoyalties(AssetListing storage asset) internal {
// calculate fees
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
​
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
​
// first, Swan receives the entire fee from seller
// this allows only one approval from the seller's side
token.transferFrom(asset.seller, address(this), buyerFee);
​
// send the buyer's portion to them
token.transfer(asset.buyer, buyerFee - driaFee);
​
// then it sends the remaining to Swan owner
token.transfer(owner(), driaFee);
}

Impact

The attacker can drain the funds of seller by collecting royaltyfee each time asset is listed and relisted, and still do not purchase any asset.

The malicious buyer then withdraw the royaltyfee, without even spending a penny he get money.

Tools Used

Manual Review

Recommendations

The following changes will transfer royalty to buyer only if the purchase is made.

  1. Remove the 2 transferRoyalties calls from list and relist function.

transferRoyalties call in Swan::list function-> https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/Swan.sol#L188

transferRoyalties call in Swan::relist function-> https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/Swan.sol#L252

2.And add the call for transferRoyaltiesin the Swan::purchasefunction as given below:-

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);
​
+ // transfer royalties
+ transferRoyalties(listings[_asset]);
​
emit AssetSold(listing.seller, msg.sender, _asset, listing.price);
}
Updates

Lead Judging Commences

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

Appeal created

saurabh_singh Submitter
12 months ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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