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

Strict equality check of the smart contract balance to the totalFees collected in `PuppyRaffle::withdrawFees` makes it vulnerable to an unexpected ether attack.

0xspryon

Summary

if a user manually sends some either at this smart contract by calling selfDestruct on another contract, this messes with the account of the fees collected in comparison to the current contract balance making it impossible for the collected fees to be withdrawn from the protocol. In other words, getting the funds locked in the smart contract.

Vulnerability Details

if a user manually sends some ether at this smart contract by calling selfDestruct on another contract, this messes with the accounting of the fees collected in comparison to the current contract balance making it impossible for the collected fees to be withdrawn from the protocol.

Impact

In other words, getting the collected fees locked in the smart contract.

Note: Because of this very same strict equality check and the fact that, the determination of the winnerIndex and prizePool in PuppyRaffle::selectWinner does not take into account that there might have been refunded users, the final ether balance left in the smart contract is not the same as PuppyRaffle::totalFees leading to the same effects explained above.

Tools Used

Manual review

Recommendations

Determine if there is an active raffle using the PuppyRaffle::players array length instead.

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