Dria

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

Loop-Based Buy Limit Check in BuyAgent Contract Causes Denial of Service (DoS) on Purchases

Summary

In the BuyAgent contract, when an authorized user attempts to make a bulk purchase, a loop checks that the cumulative price of each asset does not exceed the specified amountPerRound limit. However, if any asset's price causes the cumulative spending to surpass the buy limit, the function reverts, resulting in a DoS for subsequent assets that could have been purchased within the limit. By storing spendings[round] outside the loop and using a temporary variable to track potential cumulative spending, the contract can skip assets that would exceed the limit while allowing others to proceed.

Vulnerability Details

In the purchase function, the current implementation performs the following operations within a loop over assets:

for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
uint256 price = swan.getListingPrice(asset);
spendings[round] += price;
if (spendings[round] > amountPerRound) {
revert BuyLimitExceeded(spendings[round], amountPerRound); // @audit if revert happens all purchase fails?
}
inventory[round].push(asset);
swan.purchase(asset);
}

If spendings[round] exceeds amountPerRound for any asset, the function reverts, preventing completion of the entire purchase list.

Root Cause

The issue arises from modifying and checking spendings[round] directly within the loop, leading to a revert when the cumulative spending surpasses amountPerRound.

Impact

This flaw results in a Denial of Service (DoS) for authorized buyers, as the entire batch purchase fails if even one asset exceeds the buy limit. This stops all potential purchases within that round, negatively impacting contract functionality and user experience.

Tools Used

  • Manual Review

Recommendations

  1. Use a Temporary Variable: Store spendings[round] in a temporary variable outside the loop. Track cumulative spending within the loop and only update spendings[round] after the loop completes successfully.

  2. Conditionally Skip Assets Exceeding Limit: Check if the next asset’s price would exceed the buy limit; if so, skip that asset without reverting.

Revised example implementation:

uint256 tempSpendings = spendings[round];
for (uint256 i = 0; i < assets.length; i++) {
address asset = assets[i];
uint256 price = swan.getListingPrice(asset);
if (tempSpendings + price > amountPerRound) {
continue; // Skip assets that would exceed the buy limit
}
tempSpendings += price;
inventory[round].push(asset);
swan.purchase(asset);
}
spendings[round] = tempSpendings; // Update the actual spending only once

This approach prevents the DoS issue by enabling as many purchases as possible without breaching the limit and minimizes gas wastage due to repeated reverts.

Updates

Lead Judging Commences

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

Support

FAQs

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