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

Unused rewardPool Variable Leading to Redundant Hardcoded Rewards in MysteryBox::openBox

fels21

Summary

The rewardPool variable is initialized to hold different rewards, but it is not used in the actual reward distribution process. Instead, the rewards are hardcoded directly in the function openBox, which defeats the purpose of having a rewardPool. This makes the rewardPool variable effectively useless and suggests a design flaw in the contract.

Vulnerability Details

In the current implementation of the openBox function, the rewards are added directly as follows:

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));
}

However, there is a rewardPool variable that is initialized with the same set of rewards, which should be used in this context:

Reward[] public rewardPool;

The proposed correction is to use rewardPool for reward distribution:

if (randomValue < 75) {
rewardsOwned[msg.sender].push(rewardPool[0]);
} else if (randomValue < 95) {
rewardsOwned[msg.sender].push(rewardPool[1]);
} else if (randomValue < 99) {
rewardsOwned[msg.sender].push(rewardPool[2]);
} else {
rewardsOwned[msg.sender].push(rewardPool[3]);
}

Impact

  • Low Impact: The impact is mostly related to code maintainability and clarity. There is no direct security risk or risk to funds, but the current implementation introduces redundancy and inconsistency in how rewards are handled.

Tools Used

  • Manual code review.

Recommendations

  • Refactor the code to use the rewardPool variable for reward distribution, as originally intended. This will simplify the logic, improve maintainability, and remove unnecessary hardcoding of rewards.

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!