Vulnerability Details

The withdrawFees function currently allows the owner to withdraw the entire balance of the contract, which includes user funds that are pending a refund due to the “trick” outcome in the trickOrTreat function. As a result, users expecting to retrieve their funds by calling resolveTrick after a “trick” encounter may lose their funds if the owner has already withdrawn the balance, posing a significant risk of user fund loss.

Code: https://github.com/Cyfrin/2024-10-trick-or-treat/blob/9cb3955058cad9dd28a24eb5162a96d759bfa842/src/TrickOrTreat.sol#L146

Impact

Allowing the owner to withdraw all funds, including pending user balances, may result in irretrievable losses for users who are unable to retrieve their funds after a “trick” encounter. This significantly impacts user trust and could expose the contract to abuse by the owner.

Tools Used

Manual Review

Recommendations

Separate user pending balances from owner fees to avoid the possibility of withdrawing funds that belong to users. Track pending balances for each user individually and restrict the withdrawFees function to only withdraw the accumulated owner fees, leaving user funds untouched: