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

Incorrect Hardcoded Reward Values in `openBox` Function

4rdiii

[M-02] Incorrect Hardcoded Reward Values in openBox Function

Summary

The openBox function uses hardcoded reward values instead of fetching them from the rewardPool array. Additionally, the hardcoded values do not match the intended default reward values. This results in winners receiving larger rewards than specified.

Vulnerability Details

The hardcoded reward values in the openBox function are incorrect. According to the constructor, the correct values for rewards should be:

  • Silver Coin: 0.25 ether (currently hardcoded as 0.5 ether)

  • Gold Coin: 0.5 ether (currently hardcoded as 1 ether)

However, in the openBox function, these values are incorrect, as shown below:

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)); //@audit should be 0.25 ether
} else {
// 1% chance to get Gold Coin (99)
rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether)); //@audit should be 0.5 ether
}
​
boxesOwned[msg.sender] -= 1;
}

Impact

Due to these incorrect hardcoded values, winners are receiving higher rewards than intended, which could lead to significant financial loss over time.

Tools Used

Manual Review

Recommendations

  1. Remove hardcoded values from the openBox function and fetch the correct reward values from the rewardPool array to ensure consistency. This way, any reward values set in the constructor or added later will be properly used.

  2. Here is an example of a corrected implementation:

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 using rewardPool
if (randomValue < 75) {
// 75% chance to get Coal (0-74)
rewardsOwned[msg.sender].push(rewardPool[3]); // Coal
} else if (randomValue < 95) {
// 20% chance to get Bronze Coin (75-94)
rewardsOwned[msg.sender].push(rewardPool[2]); // Bronze Coin
} else if (randomValue < 99) {
// 4% chance to get Silver Coin (95-98)
rewardsOwned[msg.sender].push(rewardPool[1]); // Silver Coin (correct value from rewardPool)
} else {
// 1% chance to get Gold Coin (99)
rewardsOwned[msg.sender].push(rewardPool[0]); // Gold Coin (correct value from rewardPool)
}
​
boxesOwned[msg.sender] -= 1;
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year 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.

Give us feedback!