[M-01] Inability to Set Win Probability for Newly Added Rewards (e.g., Diamond Coin)
Summary
The addReward function allows the owner to add new Reward structs to the rewardPool array. However, the openBox function has fixed probabilities for default prizes, with no logic for setting or adjusting probabilities for newly added rewards.
Vulnerability Details
Below is the current implementation of the addReward and openBox functions:
function addReward(string memory _name, uint256 _value) public {
require(msg.sender == owner, "Only owner can add rewards");
rewardPool.push(Reward(_name, _value));
}
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;
}
As shown, the logic is fixed and unchangeable, rendering the addReward function ineffective for newly added rewards.
Impact
New rewards added by the owner cannot be won, since the openBox function's reward probabilities are hard-coded and not adaptable.
Tools Used
Manual Review
Recommendations
-
Consider implementing an upgradability scheme to allow modifications to the reward logic in the future.
-
Add a probability value to the Reward struct, and modify openBox to use these probability values when determining the reward. Below is a sample implementation:
struct Reward {
string name;
uint256 value;
+ uint256 probability;
}
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));
- }
+ uint256 rewardPoolLength = rewardPool.length;
+ uint256 currentProbability;
+ for (uint256 i; i < rewardPoolLength; i++){
+ // Assuming rewards in rewardPool are listed in order of rarity
+ currentProbability += rewardPool[i].probability;
+ if (randomValue < currentProbability){
+ rewardsOwned[msg.sender].push(rewardPool[i]);
+ boxesOwned[msg.sender] -= 1;
+ return;
+ }
+ }
- boxesOwned[msg.sender] -= 1;
}
