if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length)
checks if the maxAssetCount
is exactly equal to the length of the assetsPerBuyerRound
array. This means that if the length of the array is one more than the maxAssetCount
, the condition will not be met, and the function will not revert.assetsPerBuyerRound
array grows beyond the intended limit (maxAssetCount
) without triggering the AssetLimitExceeded
error. This can cause unexpected behavior, errors, or even security vulnerabilities.Attack Scenario:
An attacker creates a new buyer account and lists 5 assets using the listAsset
function.
The attacker then lists a 6th asset using the listAsset
function.
Expected Behavior:
The contract should revert with an "AssetLimitExceeded" error when the attacker tries to list the 6th asset, since the maxAssetCount
is 5.
The contract does not revert, and the attacker is able to list the 6th asset. This is because the ==
comparison in the listAsset
function only checks if the length of the assetsPerBuyerRound
array is exactly equal to the maxAssetCount
, and not greater than or equal to.
Deployment and Execution:
Deploy the Swan
contract with the maxAssetCount
set to 5.
Deploy the Attack
contract, passing the address of the Swan
contract as a constructor argument.
Call the attack
function on the Attack
contract.
The attack
function will successfully list 6 assets for the buyer, despite the maxAssetCount
being 5. This demonstrates the issue with the ==
comparison in the listAsset
function.
>=
(greater-than-or-equal-to), we ensure that the function reverts if the length of the assetsPerBuyerRound
array is greater than or equal to the maxAssetCount
. This means that if the length of the array is one more than the maxAssetCount
, the condition will be met, and the function will revert with the AssetLimitExceeded
error.Example:
Suppose maxAssetCount
is 5, and the assetsPerBuyerRound
array has 5 elements. If we add one more element to the array, its length becomes 6.
With the original code (==
), the condition would not be met, and the function would not revert.
With the updated code (>=
), the condition would be met, and the function would revert with the AssetLimitExceeded
error.
By using >=
instead of ==
, we ensure that the function behaves correctly and prevents the assetsPerBuyerRound
array from growing beyond the intended limit.
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.