Mystery Box
First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Potential Denial of Service (DoS) in `claimAllRewards` due to Gas Limit Exceedance.

v1vah0us3

Vulnerability Details

The claimAllRewards function poses a risk of Denial of Service (DoS) due to potential gas limit exhaustion. This function loops through each entry in the rewardsOwned[msg.sender] array (storage variable), which stores the user's rewards as Reward structs.

function claimAllRewards() public {
uint256 totalValue = 0;
@> for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
@> totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
​
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
​
delete rewardsOwned[msg.sender];
}

In cases where a user has accumulated a large number of rewards over time, the function will consume so much gas that it may exceed the block gas limit, making it impossible for the user to claim their rewards using claimAllRewards.

Furthermore, the associated claimSingleReward and transferReward functions compounds this problem. When users claim a single reward using claimSingleReward or transfers the reward to another user via transferReward, the functions performs a delete operation on the specific array index: delete rewardsOwned[msg.sender][_index];. However, this does not remove the array element or shift subsequent elements to fill the gap. Instead, the element is set to its default values, leaving the array size unchanged. Over time, this can result in a large number of "empty" or default entries, which further bloats the array without reducing its length.

PoC

The following gas test was designed to measure gas consumption when claiming all rewards from an array containing a total of 25,000 entries:

function testClaimAllRewards() public {
vm.deal(user1, 2500 ether);
uint256 boxPurchased = 25000;
​
vm.startPrank(user1);
for (uint i = 0; i < boxPurchased; i++) {
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.openBox();
}
vm.stopPrank();
​
uint256 gasBeforeClaim = gasleft();
vm.startPrank(user1);
vm.warp(1727529411);
mysteryBox.claimAllRewards();
vm.stopPrank();
​
uint256 gasAfterClaim = gasleft();
uint256 gasConsumed = gasBeforeClaim - gasAfterClaim;
console.log("Gas total consumed after 25000 boxes:", gasConsumed);
}

The output confirms the issue:
console::log("Gas total consumed after 25000 boxes:", 30289171) [staticcall]

Impact

The impact of this vulnerability is high because a user may accumulate so many rewards that it becomes impossible to claim them all due to gas limit exhaustion. Even though the likelihood is low (as users would have to accumulate a significant number of rewards), the consequences are severe for affected users.

Tools Used

Manual Review, VSCode, Foundry

Recommendations

An important optimization to reduce gas consumption in loops is to minimize repeated access to storage variables. By storing the value in a local variable before the loop begins, you can significantly improve efficiency and reduce gas costs:

function claimAllRewards() public {
uint256 totalValue = 0;
+ uint256 rewardsLenght = rewardsOwned[msg.sender].length;
- for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
+ for (uint256 i = 0; i < rewardsLenght; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
​
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
​
delete rewardsOwned[msg.sender];
}

To further optimize gas usage during increment operations, it's advisable to use uncheckedblocks when there is no risk of overflow, particularly in loops controlled by external logic. This eliminates the cost of Solidity's default overflow checks, providing additional gas savings.

Another possible improvement is to limit the number of rewards a user can accumulate. To do this, consider setting an upper limit on the number of rewards a user can accumulate.

Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Gas Limit Exhaustion in `claimAllRewards` Function

Support

FAQs

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

Give us feedback!