Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Malicious users can DoS legitimate sellers/buyers

kodyvim

Summary

Malicious users can DoS legitimate sellers/buyers through listing assets to exceed maxAssetCount.

Vulnerability Details

when listing assets in other to prevent OOG exception on `assetsPerBuyerRound[][]`` there's a check to prevent too many assets.
https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/Swan.sol#L168C9-L170C10

function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external
{
...snipped for brevity
// asset count must not exceed `maxAssetCount`
>>>> if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) {
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}
​
// all is well, create the asset & its listing
address asset = address(swanAssetFactory.deploy(_name, _symbol, _desc, msg.sender));
listings[asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
>>> price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
​
// add this to list of listings for the buyer for this round
>>> assetsPerBuyerRound[_buyer][round].push(asset);
​
// transfer royalties
transferRoyalties(listings[asset]);
​
emit AssetListed(msg.sender, asset, _price);
}

The issue is that malicious users can list assets with _price = 0 and revoke approval to swan contract or could actually list worthless assets, this action can be repeated till the length of the array approach maxAssetCount, therefore no other seller can list assets for that buyer within that round. There's no fee paid to the buyer since within transferRoyalties() buyerfees would be zero.
https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/Swan.sol#L260C9-L261C87

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);
}

There's no loss incured by the attacker at this point other than gasfees.

Impact

Malicious users could prevent legitimate seller from listing their assets and prevent buyers from earning royaltyfees or making valid purchase within any round.

Tools Used

Manual Review

Recommendations

Check that (asset.price * asset.royaltyFee) / 100; is non-zero or lower than certain amount.
A more complex solution would be to allow buyers to remove assets from assetsPerBuyerRound[][] and adjust its size.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

DOS the buyer / Lack of minimal amount of listing price

Support

FAQs

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