Dria

Swan
NFTHardhat
21,000 USDC
View results
Submission Details
Severity: low
Invalid

Incorrect Asset Count Limit Check Allows Exceeding Market Parameters

Summary

The list function in Swan.sol uses an incorrect comparison operator when checking the asset count limit, allowing more assets to be listed than the current market parameters allow.

Vulnerability Details

In the list function, the check for maximum asset count uses == instead of <=:

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

If the owner updates market parameters with a lower maxAssetCount than previously set, new listings can still be created even when the current count exceeds the new limit.

Example:

  1. Initial market parameters set maxAssetCount = 3

  2. Buyer is listed with 2 assets successfully (assetsPerBuyerRound[_buyer][round].length = 2)

  3. Owner updates market parameters with maxAssetCount = 1

  4. Buyer can still be listed with another asset since 2 == 1 is false, even though 2 >= 1 would be true

  5. Result: Buyer is listed with 3 assets when the current limit is 1

Impact

Unlimited listing of buyer per round

Tools Used

Manual Review

Recommendations

Recommendations Replace the equality check with a greater-than-or-equal comparison:

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

This ensures the asset count never exceeds the current market parameter limit, even after parameter updates.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[INVALID] List unlimited items

SwanManager::setMarketParameters pushes the new parameters `marketParameters.push(_marketParameters);` After that, when user calls list the protocol computes the round and the phase `(uint256 round, BuyerAgent.Phase phase,) = buyer.getRoundPhase();` Inside the getRoundPhase function you have this if statement on top: `if (marketParams.length == marketParameterIdx + 1) {`. The setMarketParameters call changed the `marketParams` length, thing which will case the first case to be false and run the else statement. At the end of that statement we see there is a new round. So the second element of this check `(getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length` is zero, because the [round] is fresh.

Support

FAQs

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