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

`MysteryBox::claimAllRewards` and `MysteryBox::claimSingleReward` functions will fail if the contract is low on balance

codedesignz

Description: The mystery box costs 0.1 ether to buy and the highest reward as per the default configuration is 0.5 ether. Additionally, owner of the contract can also draw funds out from the contract. This leads to a scenario where the reward value can be greater than the available funds. This will cause the MysteryBox::claimAllRewards and MysteryBox::claimSingleReward functions to fails.

Impact: Shortage of funds will cause MysteryBox::claimAllRewards and MysteryBox::claimSingleReward functions to fails.

Proof of Concept:

  1. Contract owner deploys the contract.

  2. User A buys a mystery box for 0.1 ether.

  3. They call MysteryBox::openBox and win a Gold Coin reward which is equivalent to 1 ether.

  4. User A proceeds to withdraw the funds by calling either MysteryBox::claimAllRewards or MysteryBox::claimSingleReward function.

  5. The call will fail as the contract currently has 0.2 ether in total. 0.1 from the contract deployment and 0.1 from the box fee that User A paid.

Recommended Mitigation: Add a check to verify that the balance of the MysteryBox contract is enough to make the payment.

function claimAllRewards() public {
uint256 totalValue = 0;
for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
+ require(address(this).balance >= totalValue, "Contract is currently low on funds. Please try to claim later");
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
delete rewardsOwned[msg.sender];
}
​
function claimSingleReward(uint256 _index) public {
require(_index <= rewardsOwned[msg.sender].length, "Invalid index");
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
+ require(address(this).balance >= value, "Contract is currently low on funds. Please try to claim later");
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
​
delete rewardsOwned[msg.sender][_index];
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Protocol should have a higher initial balance to prevent prize withdrawing problems

Support

FAQs

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

Give us feedback!