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

Inconsistent Reward Allocation Between Constructor and openBox Function

0xethsol

Vulnerability Details

In the contract, the rewards defined in the constructor and the openBox() function are inconsistent. The constructor initializes the rewardPool with four rewards:

  1. Gold Coin with a value of 0.5 ether

  2. Silver Coin with a value of 0.25 ether

  3. Bronze Coin with a value of 0.1 ether

  4. Coal with a value of 0 ether

However, when the user opens a box via the openBox() function, the possible rewards and their values are not fetched from the initialized rewardPool. Instead, the rewards are hardcoded directly into the openBox() function with slightly different values:

  1. Gold Coin with a value of 1 ether

  2. Silver Coin with a value of 0.5 ether

  3. Bronze Coin with a value of 0.1 ether

  4. Coal with a value of 0 ether

This inconsistency in reward values can lead to confusion, unexpected behavior, and possible exploitation. The expected behavior is that the rewards listed in the constructor (and potentially updated later) should match the rewards given when opening a box.

constructor() payable {
owner = msg.sender;
boxPrice = 0.1 ether;
require(msg.value >= SEEDVALUE, "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 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;
}

Impact

  1. Incorrect Reward Assignment: Users may expect rewards based on the constructor's initial setup but receive different rewards when opening a box due to the hardcoded values in the openBox() function. This could lead to a loss of user trust and confusion regarding the contract’s mechanics.

  2. Potential Exploitability: Since the reward values are hardcoded in openBox(), a malicious actor or even a legitimate owner might exploit this inconsistency by altering the constructor or reward setup while maintaining hardcoded values in openBox(). For example, the owner could set high-value rewards in the constructor to entice users, but users will receive lower rewards when opening boxes.

  3. User Mistrust: The system is opaque and confusing for users, as the contract advertises different rewards than what it actually delivers when opening a box. This discrepancy could harm the contract’s reputation and cause loss of users.

Tools Used

Manual Review

Recommendations

Unify Reward Assignment Logic: The reward values in openBox() should reference the rewardPool set in the constructor (or updated later via addReward()). Instead of hardcoding the reward values directly in openBox(), fetch rewards dynamically from the rewardPool.

function openBox() public {
require(boxesOwned[msg.sender] > 0, "No boxes to open");
​
uint256 randomValue = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender))) % 100;
​
Reward memory selectedReward;
​
if (randomValue < 75) {
// 75% chance to get Coal (0-74)
selectedReward = rewardPool[3]; // Assuming Coal is always at index 3
} else if (randomValue < 95) {
// 20% chance to get Bronze Coin (75-94)
selectedReward = rewardPool[2]; // Assuming Bronze Coin is at index 2
} else if (randomValue < 99) {
// 4% chance to get Silver Coin (95-98)
selectedReward = rewardPool[1]; // Assuming Silver Coin is at index 1
} else {
// 1% chance to get Gold Coin (99)
selectedReward = rewardPool[0]; // Assuming Gold Coin is at index 0
}
​
rewardsOwned[msg.sender].push(selectedReward);
boxesOwned[msg.sender] -= 1;
}
​
Updates

Appeal created

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

The rewards in constructor are different from the rewards in openBox

Support

FAQs

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