Root + Impact
Description
-
The MerkleAirdrop contract collects a small ETH fee from each claim and allows the owner to withdraw accumulated fees through the claimFees() function, which sends all contract ETH balance to the owner address using a low-level call.
-
The claimFees() function will permanently fail and prevent fee withdrawal when the owner is a contract that reverts on receiving ETH, consumes excessive gas in its receive/fallback function, or has no payable receive function, causing all accumulated fees to be locked in the contract forever.
function claimFees() external onlyOwner {
(bool succ,) = payable(owner()).call{ value: address(this).balance }("");
if (!succ) {
revert MerkleAirdrop__TransferFailed();
}
}
Risk
Likelihood:
-
The owner transfers contract ownership to a multi-sig wallet or DAO contract without verifying it can receive ETH
-
The owner is initially an EOA but later transfers ownership to a contract (common for protocol upgrades or DAO transitions)
-
A malicious actor gains temporary ownership and sets owner to a reverting contract, permanently locking fees
-
Smart contract wallets (Gnosis Safe, etc.) may have receive functions that consume gas unpredictably or revert under certain conditions
Impact:
-
All accumulated ETH fees become permanently locked in the contract with no recovery mechanism
-
The owner loses revenue from the 1 Gwei per claim fee across potentially thousands of claims
-
No alternative function exists to withdraw fees to a different address or with different parameters
-
The contract becomes partially broken as one of its core functions (fee collection) permanently fails
Proof of Concept
contract RevertingOwner {
receive() external payable {
revert("I don't accept ETH");
}
}
contract GasConsumingOwner {
receive() external payable {
while(true) {}
}
}
function testClaimFeesDoSWithRevertingOwner() public {
vm.deal(collectorOne, airdrop.getFee());
vm.prank(collectorOne);
airdrop.claim{value: airdrop.getFee()}(collectorOne, amountToCollect, proof);
uint256 accumulatedFees = address(airdrop).balance;
assertGt(accumulatedFees, 0);
RevertingOwner revertingOwner = new RevertingOwner();
vm.prank(airdrop.owner());
airdrop.transferOwnership(address(revertingOwner));
vm.prank(address(revertingOwner));
vm.expectRevert(MerkleAirdrop.MerkleAirdrop__TransferFailed.selector);
airdrop.claimFees();
assertEq(address(airdrop).balance, accumulatedFees);
}
function testClaimFeesDoSWithGasConsumption() public {
vm.deal(collectorOne, airdrop.getFee());
vm.prank(collectorOne);
airdrop.claim{value: airdrop.getFee()}(collectorOne, amountToCollect, proof);
GasConsumingOwner gasConsumer = new GasConsumingOwner();
vm.prank(airdrop.owner());
airdrop.transferOwnership(address(gasConsumer));
vm.prank(address(gasConsumer));
vm.expectRevert();
airdrop.claimFees();
}
Recommended Mitigation
Implement pull pattern with fee tracking (Best security)
contract MerkleAirdrop is Ownable {
using SafeERC20 for IERC20;
+ mapping(address => uint256) public feesOwed;
+ event FeesAccrued(address indexed owner, uint256 amount);
+ event FeesWithdrawn(address indexed recipient, uint256 amount);
function claim(address account, uint256 amount, bytes32[] calldata merkleProof) external payable {
if (msg.value != FEE) {
revert MerkleAirdrop__InvalidFeeAmount();
}
bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(account, amount))));
if (!MerkleProof.verify(merkleProof, i_merkleRoot, leaf)) {
revert MerkleAirdrop__InvalidProof();
}
+ feesOwed[owner()] += msg.value;
+ emit FeesAccrued(owner(), msg.value);
emit Claimed(account, amount);
i_airdropToken.safeTransfer(account, amount);
}
- function claimFees() external onlyOwner {
- (bool succ,) = payable(owner()).call{ value: address(this).balance }("");
- if (!succ) {
- revert MerkleAirdrop__TransferFailed();
- }
- }
+ function claimFees() external {
+ uint256 amount = feesOwed[msg.sender];
+ if (amount == 0) {
+ revert MerkleAirdrop__NoFeesToClaim();
+ }
+ feesOwed[msg.sender] = 0; // CEI pattern
+ (bool succ,) = payable(msg.sender).call{ value: amount }("");
+ if (!succ) {
+ feesOwed[msg.sender] = amount; // Restore on failure
+ revert MerkleAirdrop__TransferFailed();
+ }
+ emit FeesWithdrawn(msg.sender, amount);
+ }
}
