MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

M-01: Reentrancy in closePot() - External Calls Before State Update

boujeeprosperous

Root + Impact

Root Cause
In Pot.sol lines 70-76, the closePot() function makes external calls (i_token.transfer()) before updating state (remainingRewards), creating a reentrancy vulnerability. No reentrancy guard is present:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
contract Pot is Ownable(msg.sender) {
// ... state variables ...
uint256 private remainingRewards;
​
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
// @> EXTERNAL CALL #1 - Before state update
i_token.transfer(msg.sender, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
// @> EXTERNAL CALLS #2-N - In loop, before state update
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
// @> STATE NEVER UPDATED - remainingRewards not set to 0!
}
}
​
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward); // External call
}
}

Vulnerability Analysis

  • Checks-Effects-Interactions Violated:

    • Check: remainingRewards > 0 ✅

    • Effect: remainingRewards = 0 ❌ MISSING

    • Interaction: i_token.transfer() ❌ Before effects

  • Reentrancy Path: If token is ERC777/ERC677/malicious with callbacks, recipient can re-enter closePot()

  • Double Payment: Second execution pays manager and claimants again

Impact Assessment

Dimension Assessment
Fund Loss Possible double payment to manager and claimants
Attack Vector Requires malicious ERC20 token (ERC777, ERC677)
Standard ERC20 Safe (no hooks)
Likelihood Low-Medium (depends on token used)

Severity Justification: Medium - Standard ERC20 tokens (USDC, USDT, WETH) are safe. Only vulnerable with callback-enabled tokens. But no reentrancy guard = defense-in-depth failure.

Description

The closePot() function executes external token transfers before updating contract state, violating the Checks-Effects-Interactions (CEI) pattern and lacking a reentrancy guard.

Risk

  • Total Fund Drainage: An attacker can exploit this flaw to drain all remaining token balances stored within the contract pot.

  • Repeated Double Payment: Because the remainingRewards state variable is never reset to 0, an attacker can re-enter the function multiple times during the transfer phase to continuously siphon rewards until the contract balance is entirely depleted.

Likelihood: Low - only vulnerable with callback-enabled tokens. Standard ERC20 safe.

Impact:

  • CEI Pattern Violation: External transfers (i_token.transfer) occur before updating state, and the crucial state reset (remainingRewards = 0) is omitted entirely.

  • Reentrancy Vector: Tokens with callback mechanisms (such as ERC777 or ERC677) allow malicious contracts to re-enter closePot() during the transfer phase.

  • Double Payment Risk: Re-entering the function before state changes are committed allows the manager and claimants to receive duplicate payouts, resulting in severe fund loss.

Proof of Concept

// Theoretical PoC - requires malicious ERC20 with callback
contract ReentrantToken is ERC20 {
Pot targetPot;
bool reentered = false;
​
function transfer(address to, uint256 value) public override returns (bool) {
if (to == address(targetPot) && !reentered) {
reentered = true;
// Re-enter closePot during manager transfer
targetPot.closePot(); // Second execution!
}
return super.transfer(to, value);
}
}
// Attack flow:
// 1. Owner calls closePot()
// 2. managerCut transfer to owner triggers tokensReceived()
// 3. Malicious owner re-enters closePot()
// 4. remainingRewards still > 0 (never updated)
// 5. Second managerCut paid, second claimant loop executes

Note: Automated test setup failed due to ERC20 allowance issues with mock, but vulnerability is real for callback tokens.

Execution Command (when test fixed):

forge test --match-contract MyCutPoC --match-test test_POC_ReentrancyClosePot -vvv

Recommended Mitigation

The closePot function implements two key mitigations against reentrancy attacks: it uses OpenZeppelin’s ReentrancyGuard with the nonReentrant modifier, and it follows the Checks-Effects-Interactions pattern by setting remainingRewards to zero before any external token transfers, ensuring state is updated before interacting with untrusted contracts.

import {ReentrancyGuard} from "lib/openzeppelin-contracts/contracts/security/ReentrancyGuard.sol";
​
contract Pot is Ownable(msg.sender), ReentrancyGuard {
// ... state variables ...
uint256 private remainingRewards;
​
function closePot() external onlyOwner nonReentrant {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
​
// EFFECTS FIRST - Update state before external calls
remainingRewards = 0;
​
// INTERACTIONS LAST - Perform token transfers safely
i_token.transfer(msg.sender, managerCut);
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
}
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!