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

Reentrancy Vulnerability in MysteryBox::claimSingleReward Allowing Multiple Reward Claims

fels21

Summary

The claimSingleReward function in the MysteryBox contract is vulnerable to a reentrancy attack. This vulnerability allows an attacker to claim rewards multiple times before the state is updated, draining the contract's funds.

Vulnerability Details

The claimSingleReward function transfers funds to the caller using a low-level call before updating the contract's state. Specifically, it transfers the reward before deleting it from the rewardsOwned array, allowing an attacker to reenter the function and repeatedly claim the same reward.

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 reentrancy occurs because the contract transfers the reward via a low-level call before the state (i.e., deletion of the reward from the array) is updated. This enables an attacker to invoke the fallback function and reenter claimSingleReward before the state is updated, allowing multiple claims of the same reward.

Impact

An attacker can drain the contract’s funds by repeatedly calling claimSingleReward through reentrancy, claiming rewards multiple times before the state is updated. This can lead to the total loss of all funds in the contract.

Tools Used

  • Manual code review

  • Forge test suite for simulating reentrancy

Recommendations

  1. Implement the Checks-Effects-Interactions (CEI) pattern by updating the contract’s state before transferring any funds:

delete rewardsOwned[msg.sender][_index];
(bool success, ) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");

  1. Use OpenZeppelin’s ReentrancyGuard and apply the nonReentrant modifier to the claimSingleReward function to prevent reentrant calls:

function claimSingleReward(uint256 _index) public nonReentrant {
// logic here
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`claimSingleReward` reentrancy

Support

FAQs

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

Give us feedback!