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

[M-1] Unsupervised Rewards claims can cause unintended situations

player

Description: MysteryBox can grant a user to claim a box randomly and that user can open the box and claim the hidden rewards within using MysteryBox::claimAllRewards function.
This reward can vary from 1 ether to 0 ether. A user can buy this box using 0.1 ether and claim its rewards using the contracts protocols.But if a user gets a reward which is much more than the contract holds in it, then the contract fails to deliver that reward to the user.

Impact: A user can buy box using only 0.1 ether. And if he is lucky enough he gets a reward exceeding his bought amount (which is a rare case and most likely to happen only once),he will try to claim it using the MysteryBox::claimAllRewards function. But if the contract doesn't hold that much ether in it at that time, the user will surely fail to get this reward at that time and lose his trust in the protocol

Proof of Concept: Here is a proof of code given below along with its output

function test_gettingRewardEdgecase() public{
//User buys a box
vm.warp(1 days);
vm.deal(user2,5 ether);
vm.startPrank(user2);
mysteryBox.buyBox{value:0.1 ether}();
mysteryBox.openBox();
MysteryBox.Reward[] memory rewards = mysteryBox.getRewardPool();
rewards = mysteryBox.getRewards();
console2.log(rewards[0].name);
mysteryBox.claimAllRewards();
​
}

Output:

forge test --mt test_gettingRewardEdgecase -vvvv
[⠊] Compiling...
[⠑] Compiling 1 files with Solc 0.8.20
[⠘] Solc 0.8.20 finished in 1.61s
Compiler run successful!
​
Ran 1 test for test/TestMysteryBox.t.sol:MysteryBoxTest
[FAIL. Reason: revert: Transfer failed] test_gettingRewardEdgecase() (gas: 155303)
Logs:
Reward Pool Length: 4
Silver Coin
​
Traces:
[1413529] MysteryBoxTest::setUp()
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]
├─ [0] VM::label(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266], "owner")
│ └─ ← [Return]
├─ [0] VM::deal(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266], 5000000000000000000 [5e18])
│ └─ ← [Return]
├─ [0] VM::prank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [1289818] → new MysteryBox@0x88F59F8826af5e695B13cA934d6c7999875A9EeA
│ └─ ← [Return] 5312 bytes of code
├─ [6370] MysteryBox::getRewardPool() [staticcall]
│ └─ ← [Return] [Reward({ name: "Gold Coin", value: 500000000000000000 [5e17] }), Reward({ name: "Silver Coin", value: 250000000000000000 [2.5e17] }), Reward({ name: "Bronze Coin", value: 100000000000000000 [1e17] }), Reward({ name: "Coal", value: 0 })]
├─ [0] console::log("Reward Pool Length:", 4) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
​
[155303] MysteryBoxTest::test_gettingRewardEdgecase()
├─ [0] VM::warp(86400 [8.64e4])
│ └─ ← [Return]
├─ [0] VM::deal(SHA-256: [0x0000000000000000000000000000000000000002], 5000000000000000000 [5e18])
│ └─ ← [Return]
├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
│ └─ ← [Return]
├─ [24580] MysteryBox::buyBox{value: 100000000000000000}()
│ └─ ← [Stop]
├─ [68092] MysteryBox::openBox()
│ └─ ← [Stop]
├─ [24370] MysteryBox::getRewardPool() [staticcall]
│ └─ ← [Return] [Reward({ name: "Gold Coin", value: 500000000000000000 [5e17] }), Reward({ name: "Silver Coin", value: 250000000000000000 [2.5e17] }), Reward({ name: "Bronze Coin", value: 100000000000000000 [1e17] }), Reward({ name: "Coal", value: 0 })]
├─ [2130] MysteryBox::getRewards() [staticcall]
│ └─ ← [Return] [Reward({ name: "Silver Coin", value: 500000000000000000 [5e17] })]
├─ [0] console::log("Silver Coin") [staticcall]
│ └─ ← [Stop]
├─ [8198] MysteryBox::claimAllRewards()
│ ├─ [0] PRECOMPILES::sha256{value: 500000000000000000}(0x)
│ │ └─ ← [OutOfFunds]
│ └─ ← [Revert] revert: Transfer failed
└─ ← [Revert] revert: Transfer failed
​
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.03ms (191.10µs CPU time)
​

Recommended Mitigation:

  1. By making sure that the contract holds enough eth before distributing rewards will mitigate this issue.

  2. Adding a certain minimum timestamp before distributing rewards may also help to mitigate this .

Tools Used: Manual Review

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!