Dria

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

Users will be able to list unlimited items when owner changes params

Summary

Users will be able to list unlimited items when owner changes params to a lower number

Vulnerability Details

According to docs, market parameters can be changed during round

Phase & round management within Swan needs attention, especially when market parameters are to be changed by contract owner during a round.

In code there is a == isntead of >= so, whenever user hit maxAssetCount value he will not be able list more item, but when its changed by admin, e.x. it was 10 become 5, so user will be able to list unlimited items

if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) { // @audit >=?
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}

contracts/swan/Swan.sol#L168

Impact

Users will be able to list unlimited items with specific owner changes

Tools Used

Recommendations

// asset count must not exceed `maxAssetCount`
- if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) { // @audit >=?
+ if (getCurrentMarketParameters().maxAssetCount <= assetsPerBuyerRound[_buyer][round].length) { // @audit >=?
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}
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.