Mystery Box

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

Hardcoded Reward Distribution ignores dynamically added rewards

Summary

The MysteryBox contract allows new rewards to be dynamically added via the addReward function, but the openBox function uses hardcoded probabilities and reward types. This means that any new rewards added to the reward pool cannot actually be won by users opening mystery boxes. This creates a mismatch between the advertised reward pool and the rewards that are actually obtainable, potentially misleading users.

Vulnerability Details

The vulnerability stems from two functions in the MysteryBox contract:

The addReward function allows new rewards to be added to the reward pool:

function addReward(string memory _name, uint256 _value) public {
require(msg.sender == owner, "Only owner can add rewards");
rewardPool.push(Reward(_name, _value));
}

However, the openBox function uses hardcoded probabilities and reward types:

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));
@> }
boxesOwned[msg.sender] -= 1;
}

The openBox function only considers four specific rewards: Coal, Bronze Coin, Silver Coin, and Gold Coin. Any new rewards added through the addReward function are stored in the rewardPool array but are never distributed when users open boxes. This creates a discrepancy between the rewards that users can see in the reward pool and the rewards they can actually obtain.

Impact

The impact of this vulnerability is significant:

  1. Misleading Users: Users may be enticed to buy mystery boxes based on the advertised reward pool, which includes dynamically added rewards. However, they have no chance of actually winning these new rewards, potentially leading to disappointment and feelings of being misled.

  2. Economic Imbalance: If high-value rewards are added to the pool but cannot be won, it creates an artificial inflation of the perceived value of the mystery boxes without a corresponding increase in actual value distribution.

  3. Wasted Gas: The addReward function consumes gas to add new rewards to the pool, but this gas is essentially wasted since these rewards can never be distributed.

  4. Scalability Issues: As more rewards are added over time, the contract will store an increasing amount of unused data, potentially leading to increased gas costs for certain operations.

Overall, this vulnerability undermines the core functionality and fairness of the mystery box system, potentially leading to both user dissatisfaction and economic inefficiencies.

Proof of Concept

To demonstrate this vulnerability, we can add a new test to the MysteryBoxTest contract. This test will show that even after adding a new reward, users cannot receive it when opening boxes.

Add the following test function to the MysteryBoxTest contract:

function testNewRewardsNotObtainable() public {
// Add a new high-value reward
vm.prank(owner);
mysteryBox.addReward("Platinum Coin", 5 ether);
// Verify the new reward is in the pool
MysteryBox.Reward[] memory rewardPool = mysteryBox.getRewardPool();
assertEq(rewardPool[rewardPool.length - 1].name, "Platinum Coin");
assertEq(rewardPool[rewardPool.length - 1].value, 5 ether);
// Buy and open 1000 boxes
vm.deal(user1, 100 ether);
vm.startPrank(user1);
for (uint256 i = 0; i < 1000; i++) {
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.openBox();
}
vm.stopPrank();
// Check user's rewards
vm.prank(user1);
MysteryBox.Reward[] memory userRewards = mysteryBox.getRewards();
bool foundPlatinum = false;
for (uint256 i = 0; i < userRewards.length; i++) {
if (keccak256(abi.encodePacked(userRewards[i].name)) == keccak256(abi.encodePacked("Platinum Coin"))) {
foundPlatinum = true;
break;
}
}
// Assert that the user did not receive any Platinum Coin
assertFalse(foundPlatinum, "User should not have received any Platinum Coin");
// Verify that the user received other rewards
assertTrue(userRewards.length > 0, "User should have received some rewards");
}

This test does the following:

  1. Adds a new high-value "Platinum Coin" reward to the pool.

  2. Verifies that the new reward is correctly added to the reward pool.

  3. Buys and opens 1000 mystery boxes.

  4. Checks the user's rewards to see if any "Platinum Coin" was received.

  5. Asserts that no "Platinum Coin" was received, despite opening many boxes.

  6. Verifies that the user did receive other rewards, showing that the box opening mechanism is functional.

To run this test, use the following command:

forge test --match-test testNewRewardsNotObtainable -vv

the test output is attached here:

Ran 1 test for test/TestMysteryBox.t.sol:MysteryBoxTest
[PASS] testNewRewardsNotObtainable() (gas: 46479697)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 22.57ms (21.40ms CPU time)

This test will pass, demonstrating that even after adding a new reward and opening many boxes, it's impossible to receive the new reward. This proves that the reward distribution is indeed hardcoded and ignores dynamically added rewards.

Tools Used

  • Manual review of the smart contract code

  • Foundry for writing and running test cases to validate the vulnerability

Recommendations

To address this vulnerability and ensure that all rewards in the pool can be won, including newly added ones, we recommend the following approach:

  1. Dynamic Reward Distribution:
    Instead of using hardcoded probabilities and rewards, implement a system that uses the dynamic rewardPool for distribution. This ensures that all rewards, including newly added ones, have a chance of being won.

  2. Probability-based Reward System:
    Implement a probability system for rewards where the probability of winning a reward is inversely proportional to its value. This approach ensures that the most valuable rewards have the lowest probability of being won, maintaining the excitement and rarity of high-value prizes.

  3. Automatic Probability Adjustment:
    When a new reward is added to the pool, automatically adjust the probabilities of all rewards. The system should ensure that the new reward's probability is set based on its value relative to other rewards in the pool.

  4. Probability Normalization:
    After adding new rewards or adjusting probabilities, implement a normalization step to ensure that the sum of all probabilities always equals 100%.

  5. Owner Controls:
    Provide functions for the owner to manually adjust probabilities if needed, but ensure that the system maintains the inverse relationship between value and probability.

  6. Transparency:
    Implement events that log when new rewards are added or probabilities are adjusted. This increases transparency and allows users to be aware of changes to the reward pool.

By implementing these recommendations, the contract will be able to incorporate new rewards dynamically into the distribution system, maintain appropriate probabilities based on reward values, and provide a fair and exciting experience for users while preserving the rarity of high-value rewards.

Updates

Appeal created

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

addReward won't have any effect on openBox

Support

FAQs

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