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

`MysteryBox::rewardPool` is not used for reward distribution and rewards are hardcoded instead

poink

Summary

In the MysteryBox::openBox function, the rewards given to msg.sender are not retrieved from the MysteryBox::rewardPool array, but are hardcoded instead. In addition, the redefined reward values do not match the default rewards added in the constructor.

Vulnerability Details

The MysteryBox::openBox function contains no reference to MysteryBox::rewardPool:

function openBox() public {
require(boxesOwned[msg.sender] > 0, "No boxes to open");
uint256 randomValue = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender))) % 100;
if (randomValue < 75) {
rewardsOwned[msg.sender].push(Reward("Coal", 0 ether));
} else if (randomValue < 95) {
rewardsOwned[msg.sender].push(Reward("Bronze Coin", 0.1 ether));
} else if (randomValue < 99) {
rewardsOwned[msg.sender].push(Reward("Silver Coin", 0.5 ether));
} else {
rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
}
boxesOwned[msg.sender] -= 1;
}

Impact

This renders the MysteryBox::addReward and MysteryBox::getRewardPool functions useless and causes confusion about the actual value of the rewards.

Tools Used

Manual Review, Foundry

Recommendations

MysteryBox::openBox should retrieve values from the MysteryBox::rewardPool array.
The MysteryBox::addReward function should either be removed or the rarity of a prize should be calculated in a different way, such as a rarity variable in the MysteryBox::Reward struct and a redesigned MysteryBox::openBox function.

Updates

Appeal created

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

Give us feedback!