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

Lack of Input Validation in reward addition and price setting

elvin_a_block

Summary

The MysteryBox contract lacks proper input validation in two key functions: addReward and setBoxPrice. The addReward function does not validate the reward value or name, potentially allowing zero-value or improperly named rewards to be added. Similarly, setBoxPrice does not implement minimum or maximum limits, which could lead to unreasonably low or high box prices. This lack of input validation could result in unexpected behavior, economic imbalances, or user frustration.

Vulnerability Details

The vulnerability stems from two functions in the MysteryBox contract:

In the addReward function:

function addReward(string memory _name, uint256 _value) public {
require(msg.sender == owner, "Only owner can add rewards");
@> rewardPool.push(Reward(_name, _value));
}

This function allows the owner to add new rewards without any checks on the _value or _name parameters. There's no validation to ensure that:

  • The _value is greater than zero

  • The _name is not an empty string

  • The _name is unique among existing rewards

In the setBoxPrice function:

function setBoxPrice(uint256 _price) public {
require(msg.sender == owner, "Only owner can set price");
@> boxPrice = _price;
}

This function allows the owner to set any price without restrictions. There are no checks to ensure that:

  • The price is greater than zero

  • The price doesn't exceed a reasonable maximum value

These lack of validations could lead to various issues, including the creation of worthless rewards, duplicate reward names, or extreme (either too low or too high) box prices.

Impact

The lack of input validation in these functions can lead to several negative consequences:

  1. Economic Imbalance:

    • Zero-value rewards could be added, disappointing users who receive them.

    • Extremely high-value rewards could be added, potentially draining the contract of funds.

    • Box prices could be set to zero, allowing free acquisition, or set extremely high, deterring purchases.

  2. User Experience:

    • Duplicate or empty reward names could confuse users about what they've won.

    • Unreasonable box prices could frustrate users and damage the project's reputation.

  3. Contract Integrity:

    • A flood of worthless rewards could bloat the reward pool, potentially leading to gas limit issues in functions that iterate over all rewards.

    • Extreme box prices could break assumptions in other parts of the contract or in front-end applications interacting with it.

  4. Potential for Abuse:

    • A malicious or compromised owner could exploit these functions to manipulate the economy of the mystery box system.

    • Could be used in combination with other vulnerabilities (like the unrestricted owner change) to quickly drain value from the contract.

While these issues require owner access to exploit, they represent a significant risk to the overall stability, fairness, and user trust in the mystery box system. The lack of safeguards could lead to both intentional abuse and accidental misconfigurations.

Tools Used

  • Manual review of the smart contract code

Recommendations

To address the lack of input validation, we recommend implementing the following changes:

  1. For the addReward function:

function addReward(string memory _name, uint256 _value) public {
require(msg.sender == owner, "Only owner can add rewards");
+ require(bytes(_name).length > 0, "Reward name cannot be empty");
+ require(_value > 0, "Reward value must be greater than zero");
+ require(!_rewardNameExists(_name), "Reward name must be unique");
rewardPool.push(Reward(_name, _value));
}
​
+ function _rewardNameExists(string memory _name) internal view returns (bool) {
+ for (uint i = 0; i < rewardPool.length; i++) {
+ if (keccak256(bytes(rewardPool[i].name)) == keccak256(bytes(_name))) {
+ return true;
+ }
+ }
+ return false;
+ }

  1. For the setBoxPrice function:

+ uint256 public constant MAX_BOX_PRICE = 100 ether; // Set an appropriate maximum price
​
function setBoxPrice(uint256 _price) public {
require(msg.sender == owner, "Only owner can set price");
+ require(_price > 0, "Box price must be greater than zero");
+ require(_price <= MAX_BOX_PRICE, "Box price exceeds maximum allowed");
boxPrice = _price;
}

These changes will add necessary checks to prevent the addition of invalid rewards and the setting of unreasonable box prices, thereby mitigating the risks associated with the current lack of input validation.

Updates

Appeal created

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.