Summary
the `openBox` function is in charge of determining the reward that the user receives upon opening the box they bought. It does that but calculating a random number and depending on that number is the reward that the user gets. However, because of use of magic number (hard coding numbers in the function rather than saving them in a variable), the rewards are wrong for the silver and gold rewards.
Vulnerability Details
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 means that for every user that receives the silver or gold coin rewards, they get double what they were meant to get! In the constructor it is clearly not those numbers:
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));
}
Tools Used
Manual Review
Recommendations
Use constant variables instead of magic number:
+ uint256 constant GOLD_PRICE = 0.5 ether;
+ uint256 constant SILVER_PRICE = 0.25 ether;
+ uint256 constant BRONZE_PRICE = 0.1 ether;
.
SNIP
.
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));
+ rewardsOwned[msg.sender].push(Reward("Bronze Coin", BRONZE_PRICE));
} else if (randomValue < 99) {
// 4% chance to get Silver Coin (95-98)
- rewardsOwned[msg.sender].push(Reward("Silver Coin", 0.5 ether));
+ rewardsOwned[msg.sender].push(Reward("Silver Coin", SILVER_PRICE));
} else {
// 1% chance to get Gold Coin (99)
- rewardsOwned[msg.sender].push(Reward("Gold Coin", 1 ether));
+ rewardsOwned[msg.sender].push(Reward("Gold Coin", GOLD_PRICE));
}
boxesOwned[msg.sender] -= 1;
