Mystery Box

Summary

The transferReward() function in the MysteryBox contract contains a bug that leaves gaps in the rewardsOwned array after transferring a reward. This is due to the use of the delete keyword, which only sets the array element to its default value instead of removing it. This behavior results in an array with default values at certain indexes, potentially causing issues during iteration and other operations on the array.

Vulnerability Details

The transferReward() function currently performs the following steps:

Key Issue:

• The delete operation sets the reward at _index to its default value, but it does not remove the element from the array. This creates a gap in the array where the deleted element still exists, but its values are set to defaults ("" for strings, 0 for numbers).

• Arrays in Solidity remain the same size after the delete operation, which can lead to unexpected behavior when interacting with the array or iterating over it.

Example of an Exploit

Gaps in the Array Leading to Misuse:

Initial Setup: Alice has three rewards: Reward1, Reward2, and Reward3. She transfers Reward2 (at index 1) to Bob.

Execution: The contract uses the delete keyword to "remove" Reward2

Outcome: Alice's rewardsOwned array now looks like:

NB: This array still contains three elements, but one of them is a default value, creating a gap. This may cause issues when other functions operate on the array, as they may assume that all elements are valid rewards.

Impact

• Array integrity is compromised: Leaving default values in the array can cause issues with iteration, indexing, and general array handling, potentially leading to incorrect behavior in other parts of the contract.

• Operational inefficiencies: The presence of "empty" elements in the array may result in wasted gas costs when iterating over the array, or bugs if the code assumes all array elements are valid rewards.

Tools Used

Manual Review

Recommendations

Replace the element at _index with the last element in the array, then remove the last element using .pop().