Mystery Box

Vulnerability Details

Issue

The claimAllRewards function allows users to claim all their accumulated rewards in a single transaction:

Problematic Scenario:

• High Number of Rewards: If a user has accumulated a large number of rewards, the loop that sums up the total reward value (for loop) can consume a significant amount of gas.

• Exceeding Block Gas Limit: The Ethereum network imposes a maximum gas limit per block. If the gas required to process the transaction exceeds this limit, the transaction will fail.

• Unclaimable Rewards: Users are unable to claim their rewards, leading to them being effectively locked.

Why This Happens

• Linear Gas Consumption: The gas cost of the claimAllRewards function increases linearly with the number of rewards a user has.

• No Mechanism to Prevent Accumulation: The contract does not limit the number of rewards a user can hold, allowing them to accumulate an unbounded number of rewards.

Impact

• Transaction Failure: Users with a large number of rewards cannot successfully execute claimAllRewards due to gas limitations.

• Locked Rewards: Users are unable to access their rewards, leading to potential financial loss and decreased trust in the platform.

• User Frustration: The inability to claim rewards can cause dissatisfaction and harm the platform's reputation.

• Denial of Service Attack Vector: Malicious users could intentionally accumulate excessive rewards to create a DoS condition, although this is less likely due to associated costs.

Tools Used

• Manual Code Review: Analyzed the claimAllRewards function to understand its behavior with large datasets.

Recommendations

To mitigate this issue, the following strategies are recommended:

** Implement Batched Claims**

Allow users to claim rewards in smaller, manageable batches instead of all at once.

Implementation Details

• Modify claimAllRewards to Accept a Batch Size:

  function claimRewards(uint256 _batchSize) public {

  uint256 totalValue = 0;

  uint256 rewardsLength = rewardsOwned[msg.sender].length;

  require(_batchSize > 0, "Batch size must be greater than zero");

  require(rewardsLength > 0, "No rewards to claim");

  uint256 iterations = _batchSize < rewardsLength ? _batchSize : rewardsLength;

  ​

  for (uint256 i = 0; i < iterations; i++) {

  uint256 index = rewardsOwned[msg.sender].length - 1;

  totalValue += rewardsOwned[msg.sender][index].value;

  // Remove the reward

  rewardsOwned[msg.sender].pop();

  }

  require(totalValue > 0, "No rewards to claim in this batch");

  ​

  // Transfer the accumulated reward value

  (bool success,) = payable(msg.sender).call{value: totalValue}("");

  require(success, "Transfer failed");

  }

  ​