Dria

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

Selllers can grief the buyers and DOS the system

Summary

Sellers can DOS any buyer, by filling his maxAssetCount for this round with 1 wei cheap NFTs.

Vulnerability Details

Every time a seller lists an asset for a buyer to buy, they get charges a fixed percentage of the item's price:

function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external
{
// code
// transfer royalties
@> transferRoyalties(listings[asset]);
emit AssetListed(msg.sender, asset, _price);
}
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);
}

The system also implements a variable called maxAssetCount, which is used inside list to prevent sellers from spamming buyers with NFTs.

function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external
{
BuyerAgent buyer = BuyerAgent(_buyer);
(uint256 round, BuyerAgent.Phase phase,) = buyer.getRoundPhase();
if (phase != BuyerAgent.Phase.Sell) {
revert BuyerAgent.InvalidPhase(phase, BuyerAgent.Phase.Sell);
}
if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) {
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}

However that cap also enables sellers to prevent the buyers from buying anything useful, by simply filling up their slots with meaningless items worth only 1 wei in price. This is possible as the system has a check for max assets listed per buyer per round

if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) {
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}

But lacks any measure to check for the minimum fee, leaving it only a fixed percentage of the price:

function transferRoyalties(AssetListing storage asset) internal {
// calculate fees
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
token.transferFrom(asset.seller, address(this), buyerFee);
token.transfer(asset.buyer, buyerFee - driaFee);
token.transfer(owner(), driaFee);
}

Example:

  1. Game starts with 5 buyer and maxAssetCount of 10, with rounds lasting 1 day

  2. Alice doesn't like the project so she fills all buyers up to maxAssetCount with meaningless assets, with sell price of 10 wei

  3. Alice is required only to pay the gas cost (which is gonna be pennies on L2s or side-chains)

Alice effectively ruined the game for all participants:

  • Sellers cannot sell

  • Buyer cannot buy anything meaningful

  • The stories end quickly as no one is incentivized to work on them

Impact

Sellers can grief all the buyers for near 0 cost, breaking the game and worsening the experience for all players.

Tools Used

Manual review

Recommendations

Have a minimum fixed listing fee.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 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.