Submission Details
Severity:
reward can be transferred to address(0) and this is loos of funds
Summary
User can lose the rewards by sending them to empty addresses.
Vulnerability Details
The user1 has transfered its reward to address(0) and this is lost so should not happen. User1 still has 1 empty reward after he transfered his only reward which shouldn't happen but i have submitted it seperately.
function testTransferReward_AddressZero() public {
vm.deal(user1, 0.1 ether);
vm.startPrank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.openBox();
// assertEq(mysteryBox.getRewards().length, 1);
console.log("User1 Rewards Before Transfer: ", mysteryBox.getRewards().length);
mysteryBox.transferReward(address(0), 0);
console.log("User1 Rewards After Transfer: ", mysteryBox.getRewards().length);
// assertEq(mysteryBox.getRewards().length, 0);
vm.stopPrank();
vm.prank(address(0));
assertEq(mysteryBox.getRewards().length, 1);
vm.prank(address(0));
console.log("Address_Zero Rewards Before Transfer: ", mysteryBox.getRewards().length);
}
[PASS] testTransferReward_AddressZero() (gas: 125984)
Logs:
User1 Rewards Before Transfer: 1
User1 Rewards After Transfer: 1
Address_Zero Rewards Before Transfer: 1
Traces:
[130196] MysteryBoxTest::testTransferReward_AddressZero()
├─ [0] VM::deal(0x0000000000000000000000000000000000000001, 100000000000000000 [1e17])
│ └─ ← [Return]
├─ [0] VM::startPrank(0x0000000000000000000000000000000000000001)
│ └─ ← [Return]
├─ [24580] MysteryBox::buyBox{value: 100000000000000000}()
│ └─ ← [Stop]
├─ [49209] MysteryBox::openBox()
│ ├─ emit RandomValueGenerated(randomValue: 15)
│ └─ ← [Stop]
├─ [2130] MysteryBox::getRewards() [staticcall]
│ └─ ← [Return] [Reward({ name: "Coal", value: 0 })]
├─ [0] console::log("User1 Rewards Before Transfer: ", 1) [staticcall]
│ └─ ← [Stop]
├─ [49050] MysteryBox::transferReward(0x0000000000000000000000000000000000000000, 0)
│ └─ ← [Stop]
├─ [1875] MysteryBox::getRewards() [staticcall]
│ └─ ← [Return] [Reward({ name: "", value: 0 })]
├─ [0] console::log("User1 Rewards After Transfer: ", 1) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [2130] MysteryBox::getRewards() [staticcall]
│ └─ ← [Return] [Reward({ name: "Coal", value: 0 })]
├─ [0] VM::assertEq(1, 1) [staticcall]
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [2130] MysteryBox::getRewards() [staticcall]
│ └─ ← [Return] [Reward({ name: "Coal", value: 0 })]
├─ [0] console::log("Address_Zero Rewards Before Transfer: ", 1) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Impact
This way user can lose the rewards.
Tools Used
Recommendations
there should be check in the transfer reward function so that the reward can not be transfered to zero address. like this.
function transferReward(address _to, uint256 _index) public {
require(_to != address(0), "Invalid address");
require(_index < rewardsOwned[msg.sender].length, "Invalid index");
rewardsOwned[_to].push(rewardsOwned[msg.sender][_index]);
delete rewardsOwned[msg.sender][_index];
}
Rewards breakdown
nSLOC
86This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.