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

Incorrect Implementation of purchase() prevents partial purchases

bhunter

Summary

Purchase function prevents partial purchases when the cumulative spending exceeds amountPerRound, causing the function to revert entirely. This means that even if the user has sufficient funds to buy some assets within the limit, the function does not attempt to complete those purchases before hitting the limit

Vulnerability Details

The purchase() is defined as :

function purchase() external onlyAuthorized {
// check that we are in the Buy phase, and return round
(uint256 round,) = _checkRoundPhase(Phase.Buy);
​
// check if the task is already processed
uint256 taskId = oraclePurchaseRequests[round];
if (isOracleRequestProcessed[taskId]) {
revert TaskAlreadyProcessed();
}
​
// read oracle result using the latest task id for this round
bytes memory output = oracleResult(taskId);
address[] memory assets = abi.decode(output, (address[]));
​
// we purchase each asset returned
for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
​
// must not exceed the roundly buy-limit
uint256 price = swan.getListingPrice(asset);
spendings[round] += price;
if (spendings[round] > amountPerRound) {
revert BuyLimitExceeded(spendings[round], amountPerRound); //@audit this will prevent partial purchases
}
​
// add to inventory
inventory[round].push(asset);
​
// make the actual purchase
swan.purchase(asset);
}
​
// update taskId as completed
isOracleRequestProcessed[taskId] = true;
}

The for loop iterates over all assets and adds the price to spendings. Later the cumulative spendings are compared to amountPerRound. if spendings[round] > amountPerRound , the whole tx will revert. This is not correct logic,because it will prevent any user who has sufficient amount for some assets. Consider the following example:

  • Bob creates a buyerAgent with amountPerRound=8 ETH.

  • He calls Purchase function and let's say the returned assets array is [A1, A2, A3, A4, A5] with listing prices {2, 3, 5, 4, 2} ETH.

  • The first two iteration of loop will make spendings=5(2+3)

  • The third iteration will exceed the Bob's amountPerRound.(2+3+5=10). The function will revert .

The Bob supposed to buy A1 and A2 but he will not able to .
The user does not have to buy all returned assets, s/he may simply wants to buy some of the assets. Also based on mentioned example Bob could buy A1, A2 and A5.However since in the third iteration the function reverted, he will not able to get the last asset.

Impact

Partiall purchases are not possible, limiting the functionality of the codebase/protocol, disincentivizing the users

Tools Used

None. Manual Review

Recommendations

Adjust the loop so that users can buy as many assets as possible without exceeding amountPerRound. A way to achieve this in my opinion is break out of the loop once the cumulative spending exceeds amountPerRound, rather than reverting. Here is the adjusted loop:

for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
​
uint256 price = swan.getListingPrice(asset);
// Check if adding this asset would exceed the limit
if (spendings[round] + price > amountPerRound) {
break;
}
spendings[round] += price;
​
inventory[round].push(asset);
​
swan.purchase(asset);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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