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

Insufficient checks leading to invariant breaks

zealousdream572

Summary

The Swan contract prevents listing of assets past the allowed limit of maxAssetCountas seen here

// asset count must not exceed `maxAssetCount`
if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) {//@audit should check that it should not exceed as well.invariant break
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}

Therefore, it is expected that anytime a user tries to list assets that is past the allowed maxAssetCountthe call call to revert.

Vulnerability Details

The contract did not factor in a scenario where within a given round the maxAssetCountis updated by the contract owner via setMarketParametersand reduce the maxAssetCountparameter way past total assets of a user within a round.

Scenario:

  1. Let's say the maxAssetCountis set to 5 and the assetPerBuyerRoundof Bob is 4.

  2. Then within that round the maxAssetCountis updated by the client via setMarketParameterswhich reduces the maxAssetCountto 3.

  3. At this point, the maxAssetCountis 3 but the total assetPerBuyerRoundof Bob already exceeds this as it is currently 4.

  4. Bob at this point can list as many assets as possible within that given round given the check incorrectly uses == instead of >=conditionality checks.

At this point, the contract invariant would have been broken and it would allow listing past the allowed limit

Impact

The invariant of ensuring listing is within the maxAssetCountcan be broken allowing for excess listing

Tools Used

Recommendations

Consider changing the conditionality check from getCurrentMarketParameters().maxAssetCount <= assetsPerBuyerRound[_buyer][round].length)to ensure the call reverts in the event user assets are greater than the maxAssetCount

​
Updates

Lead Judging Commences

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