Summary
The openBox function doesn't match the reward pool defined in the constructor.
Vulnerability Details
https://github.com/Cyfrin/2024-09-mystery-box/blob/main/src/MysteryBox.sol#L22-L25
2024-09-mystery-box/src/MysteryBox.sol at main · Cyfrin/2024-09-mystery-box (github.com)
constructor() payable {
owner = msg.sender;
boxPrice = 0.1 ether;
require(msg.value >= SEEDVALUE, "Incorrect ETH sent");
@> 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));
}
Values declared in constructor is highlighted above.
while the values used in openBox are different as shown below:
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;
}
The reward distribution in openBox doesn't align with the reward pool initialized in the constructor, leading to inconsistent gameplay and potential economic imbalance.
It's exactly twice from the intended rewards for silver and Gold, which can be exploited by malicious users. Also unfair to legitimate players.
Impact
This mismatch can lead to an unfair distribution of rewards and potential economic imbalance in the game.
Tools Used
Manual Review
Recommendations
Align the reward distribution in openBox with the defined reward pool:
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));
+ rewardsOwner[msg.sender].push(rewardPool[3]);
} else if (randomValue < 95) {
// 20% chance to get Bronze Coin (75-94)
- rewardsOwned[msg.sender].push(Reward("Bronze Coin", 0.1 ether));
+ rewardsOwner[msg.sender].push(rewardPool[2]);
} else if (randomValue < 99) {
// 4% chance to get Silver Coin (95-98)
- rewardsOwned[msg.sender].push(Reward("Silver Coin", 0.5 ether));
+ rewardsOwner[msg.sender].push(rewardPool[1]);
} else {
// 1% chance to get Gold Coin (99)
- rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
+ rewardsOwner[msg.sender].push(rewardPool[0]);
}
boxesOwned[msg.sender] -= 1;
}
