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

The 'claimSingleReward' function will cause fail if user's reward is great than contract balance.

mengwp2004

Summary

If user's reward is great than contract balance user call 'claimSingleReward' function will cause fail.

Vulnerability Details

Contract use random to open mystery box,so contract balance may be less than user's reward.User claim reward will fail.

Impact

Proof of concepts

function testClaimSingleRewardInsufficient() public {
​
address user3 = address(0x4);
vm.deal(user1, 1 ether);
vm.startPrank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
vm.stopPrank();
​
vm.deal(user3, 1 ether);
vm.startPrank(user3);
mysteryBox.buyBox{value: 0.1 ether}();
console.log("User Buy box");
mysteryBox.openBox();
console.log("User Open box");
assertEq(mysteryBox.boxesOwned(user3), 0);
​
MysteryBox.Reward[] memory rewards = mysteryBox.getRewards();
console.log("User rewards name:",rewards[0].name);
console.log("User rewards value:",rewards[0].value);
uint256 balance = address(mysteryBox).balance;
console.log("Contract balance ",balance);
console.log("User reward balance is greater than contract balance:",rewards[0].value > balance);
mysteryBox.claimSingleReward(0);
vm.stopPrank();
}

forge test --match-test testClaimSingleRewardInsufficient -vvv
[⠊] Compiling...
[⠰] Compiling 2 files with Solc 0.8.21
[⠔] Solc 0.8.21 finished in 1.39s
Compiler run successful with warnings:
Warning (2018): Function state mutability can be restricted to view
--> test/TestMysteryBox.t.sol:295:5:
|
295 | function Attack() public {
| ^ (Relevant source part starts here and spans across multiple lines).

Ran 1 test for test/TestMysteryBox.t.sol:MysteryBoxTest
[FAIL. Reason: revert: Transfer failed] testClaimSingleRewardInsufficient() (gas: 163296)
Logs:
Reward Pool Length: 4
User Buy box
User Open box
User rewards name: Silver Coin
User rewards value: 500000000000000000
Contract balance 200000000000000000
User reward balance is greater than contract balance: true

Traces:
[163296] MysteryBoxTest::testClaimSingleRewardInsufficient()
├─ [0] VM::deal(ECRecover: [0x0000000000000000000000000000000000000001], 1000000000000000000 [1e18])
│ └─ ← [Return]
├─ [0] VM::startPrank(ECRecover: [0x0000000000000000000000000000000000000001])
│ └─ ← [Return]
├─ [24580] MysteryBox::buyBox{value: 100000000000000000}()
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::deal(Identity: [0x0000000000000000000000000000000000000004], 1000000000000000000 [1e18])
│ └─ ← [Return]
├─ [0] VM::startPrank(Identity: [0x0000000000000000000000000000000000000004])
│ └─ ← [Return]
├─ [22580] MysteryBox::buyBox{value: 100000000000000000}()
│ └─ ← [Stop]
├─ [0] console::log("User Buy box") [staticcall]
│ └─ ← [Stop]
├─ [68092] MysteryBox::openBox()
│ └─ ← [Stop]
├─ [0] console::log("User Open box") [staticcall]
│ └─ ← [Stop]
├─ [608] MysteryBox::boxesOwned(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::assertEq(0, 0) [staticcall]
│ └─ ← [Return]
├─ [2130] MysteryBox::getRewards() [staticcall]
│ └─ ← [Return] [Reward({ name: "Silver Coin", value: 500000000000000000 [5e17] })]
├─ [0] console::log("User rewards name:", "Silver Coin") [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("User rewards value:", 500000000000000000 [5e17]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Contract balance ", 200000000000000000 [2e17]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("User reward balance is greater than contract balance:", true) [staticcall]
│ └─ ← [Stop]
├─ [7951] MysteryBox::claimSingleReward(0)
│ ├─ [0] PRECOMPILES::identity{value: 500000000000000000}(0x)
│ │ └─ ← [OutOfFunds]
│ └─ ← [Revert] revert: Transfer failed
└─ ← [Revert] revert: Transfer failed

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 9.71ms (1.52ms CPU time)

Ran 1 test suite in 194.06ms (9.71ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/TestMysteryBox.t.sol:MysteryBoxTest
[FAIL. Reason: revert: Transfer failed] testClaimSingleRewardInsufficient() (gas: 163296)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Manual review.

Recommendations

If user's reward is greater than contract balance,user can claim all contract balance.

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");
​
+ if(value > address(this).balance){
+ value = address(this).balance;
+ }
(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!