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

`claimSingleReward` Function Allows `_index` Parameter to Equal rewardsOwned.length, Leading to Off-By-One Error

afdholudin

Summary

The claimSingleReward function in the MysteryBox contract has a vulnerability related to the _index parameter, which can be set to the length of the rewardsOwned array. This oversight may lead to off-by-one errors, potentially allowing users to access invalid memory locations and cause runtime errors.

Vulnerability Details

The claimSingleReward function allows the _index parameter to be equal to the length of the rewardsOwned array, which would result in an out-of-bounds access. Specifically, the condition in the require statement currently uses <=, permitting the possibility of accessing an index that does not exist in the array.

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];
}

If a user provides a value of _index equal to rewardsOwned[msg.sender].length, the function would attempt to access an invalid index, leading to runtime errors. This implementation flaw could cause the function to revert unexpectedly during execution, negatively affecting user experience.

Impact

This vulnerability allows an _index value to equal rewardsOwned[msg.sender].length, which can lead to unexpected reverts due to out-of-bounds access.

PoC

Write the following code to TestMysteryBox.t.sol:

function testClaimRewardOutOfBond() public {
vm.deal(user1, 1 ether);
vm.startPrank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.buyBox{value: 0.1 ether}();
​
mysteryBox.openBox();
mysteryBox.openBox();
mysteryBox.openBox();
​
//vm.expectRevert();
mysteryBox.claimSingleReward(3);
vm.stopPrank();
}

Output:

Ran 1 test suite in 12.03ms (1.71ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/TestMysteryBox.t.sol:MysteryBoxTest
[FAIL: panic: array out-of-bounds access (0x32)] testClaimRewardOutOfBond() (gas: 161462)

Tools Used

  • Manual review

  • Foundry

Recommendations

To mitigate the vulnerability, modify the comparison operator in the require statement from <= to <. This change ensures that the _index parameter cannot equal the length of the rewardsOwned array, thereby preventing out-of-bounds access.

function claimSingleReward(uint256 _index) public {
- require(_index <= rewardsOwned[msg.sender].length, "Invalid index");
+ 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];
}
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!