Strict == comprasion in `Swan::list` could lead to the breakdown of the invariant with maxAssetCount
Summary
Let's look at how checking that the number of listings for a particular buyer in a round is not greater than maxAssetCount works.
This check exists in two functions.
Swan::list:
And Swan::relist:
Note that in one case the strict comparison == is used, and in the other >=. Obviously, the first case may be vulnerable.
It becomes vulnerable if, during the listing phase, the Swan Owner adds new MarketParameters using the function
SwanManager::setMarketParameters.
If the maxAssetCount in the new parameters is less than the previous one, and for a given buyer - assetsPerBuyerRound[_buyer][round].length was already equal to the previous maxAssetCount - then you can add as many new listings as you want.
Vulnerability Details
Suppose that maxAssetCount = 10 and for a given buyer the number of listings is already maximal and is also 10.
Suppose that during the listing period the Swan owner calls SwanManager::setMarketParameters where he decreases maxAssetCount. Suppose it becomes equal to 8. Now assetsPerBuyerRound[_buyer][round].length = 10 and increases with each new listing. Obviously, the length == 8 check will never pass for this case - which means the transaction will not be reverted, malicious user or just other users can add as many listings as they want.
Impact
Consider the impact of breaking this invariant.
As far as I understand - information about all listings for a given buyer is passed in BuyerAgent::oraclePurchaseRequest - a call to the LLM model in the input field. During this call, the model decides which are the best assets to buy for this buyerAgent.
An incorrect length of the listing array can lead to two problems.
-
AI will process the data incorrectly and consequently give an incorrect output - as a result of which no asset will be purchased. In this case honest listings will simply lose money for the commission.
-
AI will process such a request, but will give a response, as a result of which it is more likely that the assets of users that were added after the change of the restriction will be purchased. That is, honest users are more likely not to sell their listings and therefore just lose money for the commission.
Likelihood: low
Impact: High
Severity: Medium
Tools Used
Manual Review
Recommendations
Replace strictly comparison == with >= as you did in relist.
Lead Judging Commences
[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.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.