Mystery Box

Summary

The element of the array is set to its default value of zero but array lenght remains the same.

Vulnerability Details

The functions; transferReward(), claimAllRewards() and claimSingleReward() use delete rewardsOwned[msg.sender][_index]; to remove an item from the rewardsOwned[msg.sender] array. In Solidity, delete does not reduce the length of the array. Instead, it sets the element at _index to its default value but the array length remains the same. This can lead to "gaps" in the array and doesn't fully remove the reward from msg.sender's ownership.Over time, users could accumulate many "empty" entries in their rewardsOwned array, leading to inefficient use of storage and potential confusion when accessing rewards.

https://github.com/Cyfrin/2024-09-mystery-box/blob/281a3e35761a171ba134e574473565a1afb56b68/src/MysteryBox.sol#L73

https://github.com/Cyfrin/2024-09-mystery-box/blob/281a3e35761a171ba134e574473565a1afb56b68/src/MysteryBox.sol#L79

https://github.com/Cyfrin/2024-09-mystery-box/blob/281a3e35761a171ba134e574473565a1afb56b68/src/MysteryBox.sol#L92

Impact

Gaps in storage will occur

Tools Used

Manual review

Recommendations

Replace the last element of the array with the _indexand the reduce the legth of the array using pop()