Root + Impact
Description
The collectFee()
function does not verify whether the ETH and WETH balances it transfers to the collector were actually collected through s_buyFee
payments in the buySnow()
function.
Issues:
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
@> i_weth.transfer(s_collector, collection);
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
Risk
Likelihood:
Occurs whenever ETH or WETH is sent directly to the contract via transfer.
Common in open/public contracts where many users interact with the address or spoof interactions.
Impact:
Low accounting integrity: Cannot verify that collected funds came from legitimate purchases.
Lack of auditability: Makes it hard to reconcile how much was paid and by whom.
Proof of Concept
Bob sends 100 WETH directly to the contract
Collector calls collectFee()
Contract state gives no way to trace where it came from or who paid what
Recommended Mitigation
Track Fees per User, and update buySnow and collectFee functions:
mapping(address => uint256) public wethPaid;
mapping(address => uint256) public ethPaid;
function buySnow(uint256 amount) external payable canFarmSnow {
uint256 totalCost = s_buyFee * amount;
if (msg.value == totalCost {
ethPaid[msg.sender] += msg.value;
_mint(msg.sender, amount);
} else {
i_weth.safeTransferFrom(msg.sender, address(this), totalCost);
wethPaid[msg.sender] += totalCost;
_mint(msg.sender, amount)
}
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}
function collectFee() external onlyCollector {
uint256 wethToCollect = i_weth.balanceOf(address(this));
if (wethToCollect > 0) {
i_weth.transfer(s_collector, wethToCollect);
}
uint256 ethToCollect = address(this).balance;
if (ethToCollect > 0) {
(bool collected,) = payable(s_collector).call{value: address(this).ethToCollect}("");
require(collected, "ETH collection failed!!!");
}
emit FeeCollected(msg.sender);
}