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

Both contracts inherit `Ownable` without defining a single `onlyOwner` function, leaving the self-perpetuating `s_collector` as the only privileged role, so a lost or hostile collector key permanently captures or bricks all protocol revenue

friedsocks233

Description

Snow and Snowman both inherit Ownable and both set an owner in their constructors, which signals that an administrator can intervene in the protocol.

No onlyOwner function exists in either contract — grep -rn "onlyOwner" src/ returns nothing. The owner has no powers at all. The only privileged role in the system is s_collector, and the function that rotates it is onlyCollector, so the role appoints its own successor with no oversight. s_buyFee is likewise fixed at construction with no setter.

// src/Snow.sol
@> contract Snow is ERC20, Ownable {
...
constructor(address _weth, uint256 _buyFee, address _collector) ERC20("Snow", "S") Ownable(msg.sender) {
...
@> s_buyFee = _buyFee * PRECISION;
}
...
@> function changeCollector(address _newCollector) external onlyCollector {
if (_newCollector == address(0)) {
revert S__ZeroAddress();
}
@> s_collector = _newCollector;
emit NewCollector(_newCollector);
}
// no onlyOwner function appears anywhere in src/

Risk

Likelihood:

  • Permanently and by construction: the owner is powerless from deployment onward, and the collector role is unrecoverable for the lifetime of the contract.

  • It becomes material whenever the collector key is lost, compromised, or set to a contract that cannot call changeCollector — the ordinary key-management risks that Ownable is normally present to mitigate.

Impact:

  • All accrued protocol fees are permanently captured by whoever controls the collector key, with no administrative path to rotate or revoke it.

  • If the collector key is lost, or is a contract that cannot transact, fees accumulate forever with no way to withdraw them and no way to appoint a replacement.

  • Ownable's presence is actively misleading to integrators and reviewers, who will reasonably assume an admin backstop exists. It also costs deployment gas and storage for nothing.

Proof of Concept

Paste the harness and test function(s) into test/PoC.t.sol, placing test functions inside contract PoC. Any additional helper contracts go outside contract PoC. Run forge test --match-path test/PoC.t.sol -vv.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console2} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
import {Snowman} from "../src/Snowman.sol";
import {SnowmanAirdrop} from "../src/SnowmanAirdrop.sol";
import {MockWETH} from "../src/mock/MockWETH.sol";
contract PoC is Test {
Snow snow;
Snowman nft;
MockWETH weth;
address collector = makeAddr("collector");
address attacker = makeAddr("attacker");
address victim = makeAddr("victim");
function setUp() public {
weth = new MockWETH();
snow = new Snow(address(weth), 1, collector);
nft = new Snowman("ipfs://snowman");
}
function _leaf(address who, uint256 amount) internal pure returns (bytes32) {
return keccak256(bytes.concat(keccak256(abi.encode(who, amount))));
}
// Insert the test function below here.
}
function test_CollectorIsSelfPerpetuatingAndOwnerIsPowerless() public {
assertEq(snow.owner(), address(this), "deployer IS the owner");
vm.expectRevert(Snow.S__NotAllowed.selector);
snow.changeCollector(address(0xBEEF));
address newCollector = makeAddr("newCollector");
vm.prank(collector);
snow.changeCollector(newCollector);
assertEq(snow.getCollector(), newCollector, "collector rotated itself; owner had no say");
}

Recommended Mitigation

Either give the owner the administrative powers its presence implies, or drop Ownable and stop advertising a backstop that does not exist.

- function changeCollector(address _newCollector) external onlyCollector {
+ function changeCollector(address _newCollector) external onlyOwner {
if (_newCollector == address(0)) {
revert S__ZeroAddress();
}
s_collector = _newCollector;
emit NewCollector(_newCollector);
}
+
+ function setBuyFee(uint256 _buyFee) external onlyOwner {
+ if (_buyFee == 0) {
+ revert S__ZeroValue();
+ }
+ s_buyFee = _buyFee * PRECISION;
+ }
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 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!