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

Lack of minimal amount of listing.price leads to the possibility of greafing attacks

Sickurity

Summary

Consider the function Swan::list and Swan::transferRoyalties.
Note that the list function has a limit on the number of created listings within this round - maxAssetCount.

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();
​
// buyer must be in the sell phase
if (phase != BuyerAgent.Phase.Sell) {
revert BuyerAgent.InvalidPhase(phase, BuyerAgent.Phase.Sell);
}
// 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);
}
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);
}

However, note that there is no check on the minimum _price value. Let's consider the minimum possible value - _price = 0.

At this value, the commission that the user will pay for creating a listing will obviously also be 0. This means that creating such a listing will cost the user only in payment of network commissions. Knowing that the main network of the protocol will be Base, this value is very small.

Thus, an attacker can in a single transaction take all the listings in the current round, blocking the basic functionality of the protocol for honest users.

Given that the cost of such an attack is extremely small, the attacker can do this every round and for every buyer - because the start date of a new round is a predictable value.

Vulnerability Details

The only difficulty for an attacker in carrying out this attack is that each listing has to be created from a new address.

However, this is very easy to get around, you just need to create many new contracts, each of which will call list in the constructor.

Thus the attack is really easy to perform in a single transaction.

Impact

We have the ability to block anyone from working for 1 round.
Malicious behaviour can be repeated every round.

So this is a very cheap way to intentionally DoS the protocol functionality for Buyer, or just a griefing attack.

likelihood: High

Impact: High

Severity: High

Tools Used

Manual Review

Recommendations

Add minimal amount check for _price. Note that simply checking that _price != 0 is not enough, because when _price = 1 the user's commission will also be 0 due to rounding down when dividing.

Add a check _price >= 1e18 (for example) (for ERC20 with 18 Decimals)

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.