Submission Details
Severity:
Insufficient Funds in MysteryBox Contract Causes Reward Claim Reversion
## Summary
Calling the `MysteryBox::claimAllRewards` or `MysteryBox::claimSingleReward` functions results in a `revert` due to insufficient funds in the protocol. This issue arises when the contract's balance is too low to cover the rewards distributed to players, causing the transaction to fail when players attempt to claim their rewards.
## Vulnerability Details
When the `MysteryBox` protocol is deployed, the deployer (owner) is required to seed the contract with ether greater than `SEEDVALUE`, which is 0.1 ether. However, if the deployer only sends exactly `0.1 ether` during deployment, the protocol becomes vulnerable. For instance, after deployment, a player can purchase a mystery box, open it, and receive a reward (either through frontrunning or by normal randomness) that could be valued at 0.5 ether (e.g., a "Gold Coin" as per the protocol's constructor). If the player tries to claim this reward using `MysteryBox::claimAllRewards` or `MysteryBox::claimSingleReward`, the transaction will revert because the contract lacks sufficient funds to pay the reward.
This issue could occur at any stage of the protocol, not just for a single player. If multiple players begin to win rewards larger than the balance available in the protocol, the rewards become unclaimable. The protocol only receives 0.1 ether from each player when they purchase a box, but the rewards could be significantly larger, leading to a mismatch between inflows and outflows of ether in the contract.
## Impact
This vulnerability breaks one of the core functionalities of the protocol, which is the ability for users to purchase mystery boxes, open them, receive rewards, and trade them. In cases where the protocol's balance is insufficient to cover the rewards, players are unable to claim their earnings, undermining the purpose and trust in the protocol.
Players expecting to receive rewards from opening mystery boxes will face frustration when their transactions revert due to the protocol's inability to honor the rewards. This could lead to user dissatisfaction and a loss of credibility for the protocol.
## Tools Used
The following test code has been used a proof of code to test the above said vulnerability in foundry.
<details>
<summary>code</summary>
```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
contract MysteryBoxTest is Test {
MysteryBox public mysteryBox; // Instance of the MysteryBox contract
Victim public victimPlayer; // Instance of the Victim contract representing a player who will test reward claiming
address public owner; // Address of the owner of the contract
address public user1; // Dummy user1 for testing
address public user2; // Dummy user2 for testing
// Setup function to initialize state before running tests
function setUp() public {
owner = makeAddr("owner"); // Setting up an address to represent the owner
vm.deal(owner, 100 ether); // Providing the owner with 100 ether for testing
user1 = address(0x1); // Setting up dummy user1 address
user2 = address(0x2); // Setting up dummy user2 address
// Simulate the contract deployment from the owner’s address
vm.prank(owner);
mysteryBox = new MysteryBox{value: 0.1 ether}(); // Deploy the MysteryBox contract with only 0.1 ether
// Deploy victimPlayer contract and send 1 ether to it
victimPlayer = new Victim(address(mysteryBox));
address(payable(victimPlayer)).call{value: 1 ether}(""); // Send 1 ether to the victimPlayer contract for testing
}
// Test case to simulate a player attempting to claim rewards when the protocol has insufficient funds
function testNotEnoughFunds() public {
// Capture the player's balance before the attack
uint256 balanceBefore = address(victimPlayer).balance;
// Flag to track if a favorable block timestamp is found
bool found = false;
// Start with the current block timestamp
uint256 timestamp = block.timestamp;
// Loop until we find a timestamp that produces the highest reward
while (!found) {
// Simulate the random value generation to check for a favorable outcome
uint256 pseudoRandomValue = uint256(keccak256(abi.encodePacked(timestamp, address(victimPlayer)))) % 100;
// If the random value is greater than 98, it will give the maximum reward (e.g., 0.5 or 1 ether)
if (pseudoRandomValue > 98) {
found = true; // Mark that we found the desired timestamp
} else {
timestamp++; // Increment the timestamp and check the next possible value
}
}
// Warp to the favorable timestamp (simulating block mining)
vm.warp(timestamp);
// Expect the contract to revert because it doesn't have enough ether to pay the reward
vm.expectRevert("Transfer failed");
// Simulate the victim player attempting to buy a box, open it, and claim the rewards
victimPlayer.buyAndClaimReward();
}
}
// Contract representing a player who interacts with the MysteryBox contract
contract Victim {
MysteryBox mysteryBoxInstance; // Instance of the MysteryBox contract
// Constructor to set the MysteryBox contract address
constructor(address changed) {
mysteryBoxInstance = MysteryBox(changed); // Link the deployed MysteryBox contract
}
// Function simulating the player's action: buy a box, open it, and attempt to claim rewards
function buyAndClaimReward() public {
// Buy a box from the MysteryBox contract by sending 0.1 ether
mysteryBoxInstance.buyBox{value: 0.1 ether}();
// Open the box to reveal the reward
mysteryBoxInstance.openBox();
// Attempt to claim all rewards earned from the opened box
mysteryBoxInstance.claimAllRewards();
}
// Fallback function to receive ether from the contract
receive() external payable {}
}
```
</details>
## Recommendations
In order to mitigate this issue, the protocol needs a series of changes.I will list the changes needed as concepts and then at the end provide the corresponding changes needed in the whole protocol's code at the end.
1. Maximum number of players who can play the game can be fixed through constructor and a separate function can be made to adjust the same, but only by owner.
2. Track the number of players who played the game and not allow more players than the fixed max numebr of players fixed by the owner.
3. Maintain enough funds in the protocol, as much as the edge case requires, which is every player wins the maximum reward. So , if the maximum reward a player can recevie is `x ether` and the maximum number of players is `n`, the owner must send and maintain `nx ether` inside the protocol so that it never goes out of funds in any scenario.
```diff
+ uint256 maxNumberOfPlayers;
+ uint256 numberOfPlayersPlayed;
constructor() payable {
owner = msg.sender;
boxPrice = 0.1 ether;
//value 10 is used for illustration only
+ maxNumberOfPlayers = 10;
- require(msg.value >= SEEDVALUE, "Incorrect ETH sent");
+ require(msg.value >= maxNumberOfPlayers*0.5 ether, "Incorrect ETH sent");
// Initialize with some default rewards
rewardPool.push(Reward("Gold Coin", 0.5 ether));
rewardPool.push(Reward("Silver Coin", 0.25 ether));
rewardPool.push(Reward("Bronze Coin", 0.1 ether));
rewardPool.push(Reward("Coal", 0 ether));
}
+ function changeMaxNumberOfPlayers(uint256 newMaxNumberOfPlayers) public payable {
+ require(msg.sender == owner, "Only owner can set price");
+ uint256 currentProtocolBalance = address(this).balance;
+ uint256 maxValue=0;
+ for (uint256 i = 0; i < rewardPool.length; i++) {
+ if (rewardPool[i].value > maxValue) {
+ maxValue = rewardPool[i].value;
+ }
+ }
+ uint256 balanceNeededForNewMaxNumberOfPlayers = maxValue*newMaxNumberOfPlayers;
+ if(balanceNeededForNewMaxNumberOfPlayers>currentProtocolBalance) {
+ require(msg.value>=balanceNeededForNewMaxNumberOfPlayers-currentProtocolBalance,"More Funds to be sent");
+ }
+ }
function buyBox() public payable {
+ require(numberOfPlayersPlayed<maxNumberOfPlayers,"Maximum number of players have played");
require(msg.value == boxPrice, "Incorrect ETH sent");
boxesOwned[msg.sender] += 1;
}
```
Updates
Appeal created
inallhonesty 11 months ago
Submission Judgement Published Assigned finding tags:
Protocol should have a higher initial balance to prevent prize withdrawing problems
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.