Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: low
Likelihood: medium
Invalid

Reentrancy Vulnerability in collectFee Function of Snow.sol

mohamed2__1

The collectFee function in Snow.sol performs external calls (transfer and call) to send the contract’s entire WETH and ETH balance to s_collector before updating any state variables or performing checks (aside from the onlyCollector modifier). Currently, the function does not modify any internal state variables, so a reentrancy attack cannot cause direct financial loss. However, the pattern violates the Checks-Effects-Interactions principle and creates a dangerous precedent: if any state variable (e.g., a fee counter, a withdrawal lock, or a rate limiter) is added in the future without moving it before the external call, the contract will become critically vulnerable.



function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection); // External call 1
(bool collected,) = payable(s_collector).call{
value: address(this).balance
}(""); // External call 2
require(collected, "Fee collection failed!!!");
}

** Proof of Concept (PoC)**

An attacker can deploy a malicious contract set as s_collector that re-enters collectFee() during the ETH transfer.

Malicious Collector Contract:

solidity

contract MaliciousCollector {
Snow public snow;
​
constructor(address _snow) {
snow = Snow(_snow);
}
​
// Called when ETH is received from Snow
receive() external payable {
// Re-enter collectFee to disrupt logic or exploit any future state
if (address(snow).balance > 0) {
snow.collectFee();
}
}
}

Steps:

  1. Deploy MaliciousCollector with the address of Snow.

  2. Set the collector in Snow to the malicious contract (requires governance approval or exploit of the setter).

  3. Someone calls collectFee() legitimately. The function sends ETH to MaliciousCollector, its receive() triggers collectFee() again.

  4. Currently, the second call does nothing harmful because balances are zero. However, if a state variable like totalFeesCollected were decremented after the call, the attacker could drain extra funds or bypass limits.


Recommended Mitigation

- remove this code
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection); // External call 1
(bool collected,) = payable(s_collector).call{
value: address(this).balance
}(""); // External call 2
require(collected, "Fee collection failed!!!");
}
​
​
​
+ add this code
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
function collectFee() external onlyCollector nonReentrant {
// Checks
uint256 wethBalance = i_weth.balanceOf(address(this));
uint256 ethBalance = address(this).balance;
require(wethBalance > 0 || ethBalance > 0, "Nothing to collect");
​
// Effects: (none currently, but add any state updates here in the future)
// e.g., lastCollectionTime = block.timestamp;
​
// Interactions
if (wethBalance > 0) {
i_weth.transfer(s_collector, wethBalance);
}
if (ethBalance > 0) {
(bool success,) = payable(s_collector).call{value: ethBalance}("");
require(success, "ETH transfer failed");
}
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!