Mystery Box

Summary

Improper array deletion in the transferReward, claimAllRewards, and claimSingleReward functions.

Vulnerability Details

The issue stems from the way array elements are deleted. In the contract, the delete keyword is used to remove elements, which only resets the value of the array item to its default value rather than completely removing the element from the array. For example, deleting an address will reset it to the zero address, deleting a string will reset it to an empty string, and deleting a uint will reset it to zero.

Impact

Arrays become bloated with "dust" items (i.e., default values), leading to excessive array length. When functions that loop through these arrays are called, they consume more gas due to the additional empty or default elements. This results in inefficient gas usage and can slow down contract operations over time.

Tools Used

Manual code review.

Recommendations

To address this issue, implement proper array deletion by shifting elements and reducing the array length rather than just resetting values. This ensures that elements are permanently removed, avoiding bloated arrays.
This example provides better way of array deletion : https://github.com/SunWeb3Sec/DeFiVulnLabs/blob/main/src/test/Array-deletion.sol