Mystery Box

Summary

The transferReward and reward deletion logic uses the delete operation on array elements, which can lead to inefficient reward management and potential issues with array integrity. This approach leaves gaps in the array, which can cause unexpected behavior and increased gas costs.

Vulnerability Details

In Solidity, the delete operation zeros out the contents of an element at a specified index in a dynamic array but does not change the array's length. This can lead to storage inefficiencies because the deleted element still occupies space, even though it no longer holds meaningful data.

Consider the following scenario in the transferReward and claimSingleReward functions of a smart contract:

delete does not remove the element from the array. Instead, it resets the value of the Reward struct at the specified index to an empty state, where name becomes an empty string and value becomes 0.

Impact

Arrays with deleted elements require additional gas during operations because the empty slots still need to be traversed or checked. Since delete does not reduce the array length, subsequent loops (e.g., in the claimAllRewards function) will continue to iterate over "empty" slots, leading to unnecessary gas consumption.
Deleted elements that leave behind empty slots take up storage space unnecessarily. Over time, this can contribute to bloated arrays, which can increase the cost of future interactions with the contract.

Tools Used

Manual Review

Recommendations

Instead of using delete to remove an element from the array, shift the elements after the deleted one to the left, and then use pop to reduce the array's length. This method ensures that the array remains compact, with no empty elements, and reduces the gas costs for future operations.