Mystery Box
First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Boundary Breach: Off-by-One Error in Reward Claim Function

juggernaut63

Summary

The claimSingleReward function in the MysteryBox contract contains an off-by-one error in its index validation logic. This allows users to attempt accessing an out-of-bounds index, potentially leading to unexpected behavior or contract failure.

Vulnerability Details

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

The function incorrectly validates the index using <= instead of <, allowing an index equal to the length of the array, which is out of bounds.

function claimSingleReward(uint256 _index) public {
@=> require(_index <= rewardsOwned[msg.sender].length, "Invalid index");
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
​
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
​
delete rewardsOwned[msg.sender][_index];
}

The line require(_index <= rewardsOwned[msg.sender].length, "Invalid index"); is where the off-by-one error occurs, allowing an invalid index to pass the check.

Impact

  • Attempting to access an out-of-bounds index could cause the contract to revert, potentially disrupting service for legitimate users.

Tools Used

Manual review

Recommendations

Ensure the index is strictly less than the length of the array to prevent out-of-bounds access.

function claimSingleReward(uint256 _index) public {
require(_index < rewardsOwned[msg.sender].length, "Invalid index"); // Corrected validation
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
​
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
​
- delete rewardsOwned[msg.sender][_index];
​
// Consider reordering the array to maintain integrity
+ uint256 lastIndex = rewardsOwned[msg.sender].length - 1;
+ rewardsOwned[msg.sender][_index] = rewardsOwned[msg.sender][lastIndex];
+ rewardsOwned[msg.sender].pop();
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!