MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Submission Details
Severity: high
Valid

MyCut — Smart Contract Audit Report

josephyala

MyCut — Smart Contract Audit Report

Executive Summary

Client: MyCut Protocol
Audit Date: [Date]
Report Version: 1.0

Overview

MyCut is a contest-rewards distribution protocol where owners create reward pools for contests, players claim their shares within 90 days, and unclaimed funds are redistributed with a 10% management fee.

Critical Findings at a Glance

The audit identified 4 critical High-severity vulnerabilities that result in permanent fund locks and protocol revenue loss, 4 Medium-severity issues affecting functionality and fund safety, and 3 Low-severity concerns for operational robustness.

Highest Priority: The combination of H-1, H-2, and H-3 creates a scenario where:

  • Player funds are permanently locked in the pot

  • Protocol fees are permanently locked in the manager

  • Late claimants can double-spend the pot

Project Scope

Contract nSLOC Description
src/Pot.sol ~78 Individual contest reward pool management
src/ContestManager.sol ~61 Contest creation and lifecycle management

Findings Summary

ID Severity Title
[H-1] High Forfeited rewards are never fully redistributed — wrong denominator locks funds permanently
[H-2] High Manager cut is sent to ContestManager which has no withdrawal function — fees permanently locked
[H-3] High No claim deadline — post-close claims double-spend the pot and permanently lock late claimants out
[H-4] High closePot is re-callable and drains the entire pot
[M-1] Medium No validation that totalRewards >= sum(rewards) — claimCut underflows, locking the last claimants
[M-2] Medium Division by zero when a pot is created with zero players — pot can never be closed
[M-3] Medium No "already funded" guard — double-funding permanently locks tokens
[M-4] Medium No input validation in createContest (array-length mismatch, duplicates)
[L-1] Low Integer-division dust is permanently locked (no rescue function)
[L-2] Low ERC20 transfer/transferFrom return values are unchecked
[L-3] Low No emergency-withdraw/rescue path for any error mode

Detailed Findings

HIGH SEVERITY VULNERABILITIES

[H-1] Forfeited Rewards Are Never Fully Redistributed — Wrong Denominator Locks Funds Permanently

Severity: High
Category: Loss of Funds (Permanent Lock)
Affected Code: src/Pot.sol:57

Vulnerability Details

The post-close redistribution divides the forfeited pool by the total number of players instead of the number of claimants:

uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;

The Issue: Per the protocol specification ("the remainder is distributed equally to those who claimed in time"), every token a non-claimant forfeits should go to the claimants. Dividing by i_players.length reserves a share for players who never claimed and can never claim, leaving that share permanently locked in the pot.

Impact Analysis

Scenario Tokens Locked
3 players, 1 doesn't claim ~33% of remaining pool locked
10 players, 3 don't claim ~30% of remaining pool locked
n players, m claimants (remaining - managerCut) * (players - claimants) / players locked

Mathematical Example:

  • 3 players with 1000 tokens each (total 3000)

  • Alice and Bob claim; Carol does not → remainingRewards = 1000

  • Manager cut (10%): 100 tokens

  • Current implementation: ClaimantCut = 900/3 = 300 each

  • Result: 300 tokens permanently locked in pot

  • Expected: ClaimantCut = 900/2 = 450 each, pot empty

Proof of Concept

function testPoC_WrongDenominator_LockedFunds() public {
// Setup: 3 players, 1000 each, total funded 3000
address alice = address(0x1);
address bob = address(0x2);
address carol = address(0x3);
address[] memory players = new address[](3);
players[0] = alice;
players[1] = bob;
players[2] = carol;
uint256[] memory rewards = new uint256[](3);
rewards[0] = 1000 ether;
rewards[1] = 1000 ether;
rewards[2] = 1000 ether;
// Create and fund pot
uint256 contestId = contestManager.createContest(players, rewards, 3000 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Fast forward 90 days
vm.warp(block.timestamp + 90 days);
// Alice and Bob claim
vm.prank(alice);
pot.claimCut();
vm.prank(bob);
pot.claimCut();
// Close pot
contestManager.closeContest(contestId);
// Verify: 300 tokens remain permanently locked
uint256 remaining = rewardToken.balanceOf(potAddr);
assertEq(remaining, 300 ether); // Funds permanently locked
}

Recommended Mitigation

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days)
revert Pot__StillOpenForClaim();
uint256 remainingRewards = i_totalRewards - _getClaimedRewards(); // Track claimed rewards
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / 10; // 10%
i_token.transfer(owner(), managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

[H-2] Manager Cut Is Sent to ContestManager Which Has No Withdrawal Function — Fees Permanently Locked

Severity: High
Category: Loss of Protocol Revenue
Affected Code: src/Pot.sol:55, src/ContestManager.sol:53-60

Vulnerability Details

Pot is declared as Ownable(msg.sender). When deployed from ContestManager.createContest(), msg.sender — and therefore the Pot's owner — is the ContestManager contract itself.

When the owner calls ContestManager.closeContest(), it calls pot.closePot(). Inside closePot():

i_token.transfer(msg.sender, managerCut);

The Issue: msg.sender is the ContestManager contract (not the owner EOA). ContestManager has no withdrawal/sweep/rescue function, so the 10% fee is permanently stuck in the ContestManager contract.

Impact Analysis

Scenario Impact
Any contest with unclaimed funds Protocol revenue permanently lost
Every closed contest Manager cut sent to dead-end contract
Protocol economics Fee collection mechanism completely broken

Proof of Concept

function testPoC_ManagerCutLockedInManagerContract() public {
// Setup: 2 players with 500 each
address alice = address(0x1);
address bob = address(0x2);
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 500 ether;
rewards[1] = 500 ether;
// Create and fund contest
uint256 contestId = contestManager.createContest(players, rewards, 1000 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Alice claims
vm.prank(alice);
pot.claimCut();
// Fast forward 90 days
vm.warp(block.timestamp + 90 days);
// Track balances before close
uint256 managerBefore = rewardToken.balanceOf(address(contestManager));
// Close contest
contestManager.closeContest(contestId);
// Verify: Fee is stuck in ContestManager
uint256 managerAfter = rewardToken.balanceOf(address(contestManager));
assertEq(managerAfter - managerBefore, 50 ether); // 10% of 500
// Verify: Owner receives nothing
uint256 ownerBalance = rewardToken.balanceOf(owner);
assertEq(ownerBalance, 10000 ether - 1000 ether); // Original balance minus funded amount
}

Recommended Mitigation

Send the cut to the actual owner instead of msg.sender:

// Option 1: Use Ownable owner
i_token.transfer(owner(), managerCut); // via Ownable
​
// Option 2: Add withdrawal to ContestManager
function withdrawFees(IERC20 token, uint256 amount) external onlyOwner {
token.transfer(owner(), amount);
}
​
// Option 3: Make Pot Ownable properly
constructor(address owner, ...) Ownable(owner) {
// ...
}

[H-3] No Claim Deadline — Post-Close Claims Double-Spend the Pot and Permanently Lock Late Claimants

Severity: High
Category: Loss of Funds, Double-Spend Vulnerability
Affected Code: src/Pot.sol:37-47, src/Pot.sol:49-62

Vulnerability Details

claimCut() never checks the 90-day deadline:

function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) revert Pot__RewardNotFound();
playersToRewards[player] = 0;
remainingRewards -= reward; // still credited in full
claimants.push(player);
_transferReward(player, reward);
}

Critical Issues:

  1. No deadline check - Players can claim anytime, even after closePot

  2. Post-close claims double-spend - closePot distributes forfeited shares, then late players claim their full original reward

  3. Players not marked settled - closePot doesn't zero playersToRewards for unclaimed players

  4. First-come-first-served race - Once pot balance is exhausted, legitimate claimants are permanently locked out

Attack Scenario

Setup: 3 players, 1000 each, total 3000. Only Alice claims before deadline.

Step Operation Pot Balance Issues
1 Alice claims 1000 2000 Legitimate claim
2 closePot called 1200 Manager: 200, Alice gets 600
3 Bob claims 1000 200 Invalid claim (should have forfeited)
4 Carol claims 1000 Reverts Permanently locked out of 1000

Impact Analysis

  • Double Payment: Forfeited shares paid to claimants at close, then again to late claimants

  • Unfair Outcomes: Payment results become order-dependent

  • Permanent Lock: Last legitimate claimants can never receive their rewards

  • No Safe Terminal State: Pot never reaches zero balance

Proof of Concept

function testPoC_ClaimAfterClosePot() public {
// Setup: 3 players, 1000 each
address alice = address(0x1);
address bob = address(0x2);
address carol = address(0x3);
address[] memory players = new address[](3);
players[0] = alice;
players[1] = bob;
players[2] = carol;
uint256[] memory rewards = new uint256[](3);
rewards[0] = 1000 ether;
rewards[1] = 1000 ether;
rewards[2] = 1000 ether;
// Create and fund
uint256 contestId = contestManager.createContest(players, rewards, 3000 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Alice claims before deadline
vm.prank(alice);
pot.claimCut();
// Fast forward 90 days
vm.warp(block.timestamp + 90 days);
// Close pot - redistributes to Alice
contestManager.closeContest(contestId);
// Bob claims after close - gets full 1000 (should be forfeited)
vm.prank(bob);
pot.claimCut(); // Bob gets 1000 tokens
// Carol tries to claim - REVERTS! (pot has insufficient balance)
vm.prank(carol);
vm.expectRevert(); // Transfer fails
pot.claimCut();
// Carol's 1000 is permanently lost
assertEq(rewardToken.balanceOf(carol), 0);
}

Recommended Mitigation

bool private s_closed;
uint256 private s_deadline;
​
constructor(
address owner,
address[] memory players,
uint256[] memory rewards,
uint256 totalRewards
) {
// ... existing code ...
s_deadline = block.timestamp + 90 days;
s_closed = false;
}
​
function claimCut() public {
if (s_closed) revert Pot__AlreadyClosed();
if (block.timestamp >= s_deadline) revert Pot__ClaimPeriodEnded();
// ... rest of claim logic ...
}
​
function closePot() external onlyOwner {
if (block.timestamp < s_deadline) revert Pot__StillOpenForClaim();
if (s_closed) return;
s_closed = true;
// ... distribution logic ...
// Clear all unclaimed rewards
for (uint256 i = 0; i < i_players.length; i++) {
if (playersToRewards[i_players[i]] > 0) {
playersToRewards[i_players[i]] = 0;
}
}
}

[H-4] closePot is Re-Callable and Drains the Entire Pot

Severity: High
Category: Loss of Funds
Affected Code: src/Pot.sol:49-62, src/ContestManager.sol:53-60

Vulnerability Details

closePot() performs no state transition:

  • Never sets a closed flag

  • Never updates remainingRewards

  • No validation that pot hasn't been closed before

Attack Vector: The owner (or anyone able to reach closePot) can call it repeatedly. Each call:

  1. Takes another 10% of unchanged remainingRewards

  2. Pays out claimant cuts again

  3. Does not decrease the pool proportionally (wrong denominator issue compounds)

Mathematical Drain

Repeated calls converge to ~100% of the pot drained to ContestManager:

# Calls Manager Cut (cumulative) Remaining in Pot
1 100 900
2 190 810
3 271 729
... ... ...
10 ~651 ~0

Impact Analysis

  • Complete Pot Drain: Manager can extract nearly all tokens

  • Dead-End Funds: Every drained token goes to ContestManager (no withdrawal)

  • Compound Issue: Combined with H-2, fees are permanently lost

  • Repeated Claimant Payouts: Claimants receive their (incorrectly computed) cut each call

Proof of Concept

function testPoC_ClosePotReCallable_DoubleCut() public {
// Setup: 1 player, 1000 reward
address player = address(0x1);
address[] memory players = new address[](1);
players[0] = player;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 1000 ether;
uint256 contestId = contestManager.createContest(players, rewards, 1000 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Fast forward 90 days
vm.warp(block.timestamp + 90 days);
// Call closePot 10 times
for (uint i = 0; i < 10; i++) {
contestManager.closeContest(contestId);
}
// Verify: Pot is nearly drained to ContestManager
uint256 managerBalance = rewardToken.balanceOf(address(contestManager));
uint256 potBalance = rewardToken.balanceOf(potAddr);
assertGt(managerBalance, 650 ether); // ~65% drained
assertLt(potBalance, 350 ether); // ~35% remaining
}

Recommended Mitigation

Make closing idempotent and distribute actual balance:

bool private s_closed;
​
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days)
revert Pot__StillOpenForClaim();
if (s_closed) return; // or revert Pot__AlreadyClosed();
s_closed = true;
uint256 balance = i_token.balanceOf(address(this));
if (balance == 0) return;
// Calculate cuts against actual balance
uint256 managerCut = balance / 10;
uint256 remainingAfterCut = balance - managerCut;
i_token.transfer(owner(), managerCut);
if (claimants.length > 0) {
uint256 claimantCut = remainingAfterCut / claimants.length;
uint256 dust = remainingAfterCut % claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
// Send any dust to owner
if (dust > 0) {
i_token.transfer(owner(), dust);
}
}
}

MEDIUM SEVERITY VULNERABILITIES

[M-1] No Validation That totalRewards >= sum(rewards) — claimCut Underflows

Severity: Medium
Category: Permanent Lock, Panic Revert
Affected Code: src/Pot.sol:22-35, src/Pot.sol:44

Vulnerability Details

createContest accepts rewards and totalRewards independently. The constructor stores remainingRewards = totalRewards with no check that sum(rewards) <= totalRewards.

claimCut() does remainingRewards -= reward, which underflows (Solidity Panic 0x11) as soon as the running total of claims exceeds the funded amount.

Impact Analysis

Funding Error Result
totalRewards < sum(rewards) First claimants succeed, remaining revert
Underfunding by 10% Last 10% of claimants permanently locked out
No recovery mechanism Trapped rewards never recoverable

Proof of Concept

function testPoC_Underflow_LastClaimantLocked() public {
// Setup: 2 players, 100 each, but totalRewards = 150 (underfunded)
address alice = address(0x1);
address bob = address(0x2);
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100 ether;
rewards[1] = 100 ether;
// totalRewards is 150 < 200
uint256 contestId = contestManager.createContest(players, rewards, 150 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Alice claims - succeeds
vm.prank(alice);
pot.claimCut(); // remainingRewards: 150 -> 50
// Bob claims - UNDERFLOW! Panic(0x11)
vm.prank(bob);
vm.expectRevert("Panic(0x11)"); // Underflow error
pot.claimCut();
// Bob's 100 is permanently lost
assertEq(rewardToken.balanceOf(bob), 0);
}

Recommended Mitigation

function createContest(
address[] memory players,
uint256[] memory rewards,
uint256 totalRewards
) external onlyOwner returns (uint256) {
require(players.length == rewards.length, "Length mismatch");
require(players.length > 0, "No players");
uint256 sumRewards;
for (uint256 i = 0; i < rewards.length; i++) {
sumRewards += rewards[i];
}
require(sumRewards == totalRewards, "Total rewards mismatch");
// ... rest of function
}

[M-2] Division by Zero When a Pot Is Created With Zero Players

Severity: Medium
Category: Permanent Lock
Affected Code: src/Pot.sol:57

Vulnerability Details

createContest allows an empty players array. At close, claimantCut = (remainingRewards - managerCut) / i_players.length divides by 0 → Panic(0x12), reverting the entire transaction.

Impact Analysis

  • Complete Fund Lock: Entire pot becomes unrecoverable

  • All-or-Nothing Revert: Even manager's 10% cut cannot be withdrawn

  • No Recovery: Tokens permanently locked in pot

Proof of Concept

function testPoC_DivisionByZero_NoPlayers() public {
// Create contest with empty players array
address[] memory players = new address[](0);
uint256[] memory rewards = new uint256[](0);
uint256 contestId = contestManager.createContest(players, rewards, 1000 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Fast forward 90 days
vm.warp(block.timestamp + 90 days);
// closePot reverts with division by zero
vm.expectRevert("Panic(0x12)");
contestManager.closeContest(contestId);
// All 1000 tokens remain locked in pot
assertEq(rewardToken.balanceOf(potAddr), 1000 ether);
}

Recommended Mitigation

// In createContest
require(players.length > 0, "No players");
​
// In closePot
if (claimants.length > 0) {
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
// ... distribute
}

[M-3] No "Already Funded" Guard — Double-Funding Permanently Locks Tokens

Severity: Medium
Category: Loss of Funds
Affected Code: src/ContestManager.sol:28-38

Vulnerability Details

fundContest() transfers totalRewards from the owner to the pot every time it's called. Nothing records that a contest has already been funded.

Impact Analysis

Scenario Impact
Owner funds twice Excess tokens permanently locked
After closePot exhausted Re-funding sends tokens to dead pot
No recovery mechanism Surplus tokens unrecoverable

Proof of Concept

function testPoC_DoubleFundingLocksTokens() public {
// Setup: 1 player, 200 reward
address player = address(0x1);
address[] memory players = new address[](1);
players[0] = player;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 200 ether;
uint256 contestId = contestManager.createContest(players, rewards, 200 ether);
address potAddr = contestManager.getContest(contestId);
Pot pot = Pot(potAddr);
// Fund twice
contestManager.fundContest(contestId);
contestManager.fundContest(contestId); // Double funding
// Player claims their 200
vm.prank(player);
pot.claimCut(); // remainingRewards = 0
// 200 tokens remain permanently locked in pot
uint256 potBalance = rewardToken.balanceOf(potAddr);
assertEq(potBalance, 200 ether); // Excess funds locked
}

Recommended Mitigation

mapping(address => bool) public contestToFunded;
​
function fundContest(uint256 index) public onlyOwner {
address potAddr = getContest(index);
require(!contestToFunded[potAddr], "Already funded");
contestToFunded[potAddr] = true;
// ... rest of function
}

[M-4] No Input Validation in createContest

Severity: Medium
Category: Configuration Error, Silent Misbehavior
Affected Code: src/ContestManager.sol:16-26, src/Pot.sol:32-34

Vulnerability Details

createContest does not validate:

  1. players.length == rewards.length → Opaque Panic(0x32) or silent errors

  2. Non-zero addresses → Zero address allocations

  3. Duplicate players → Earlier allocations overwritten

  4. sum(rewards) == totalRewards → See M-1

Impact Analysis

Validation Missing Result
Array length mismatch Opaque revert or ignored tail entries
Duplicate addresses Earlier rewards vanish
Zero addresses Funds sent to address(0)
Sum mismatch Underflow and permanent lock

Recommended Mitigation

function createContest(
address[] memory players,
uint256[] memory rewards,
uint256 totalRewards
) external onlyOwner returns (uint256) {
// Length validation
require(players.length == rewards.length, "Length mismatch");
require(players.length > 0, "No players");
// Sum validation
uint256 sumRewards;
for (uint256 i = 0; i < rewards.length; i++) {
sumRewards += rewards[i];
}
require(sumRewards == totalRewards, "Total mismatch");
// Duplicate and zero address validation
for (uint256 i = 0; i < players.length; i++) {
require(players[i] != address(0), "Zero address");
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "Duplicate player");
}
}
// ... create pot
}

LOW SEVERITY VULNERABILITIES

[L-1] Integer-Division Dust Is Permanently Locked

Severity: Low
Category: Minor Fund Lock
Affected Code: src/Pot.sol:57

Vulnerability Details

(remainingRewards - managerCut) / claimants.length truncates. Any remainder (< claimants.length) stays in the pot with no way out.

Proof of Concept

function testPoC_Dust_LockedRemainder() public {
// Setup: 3 players, 1009 tokens total (not divisible by 3)
// ... create pot ...
// After distribution, 9 tokens remain permanently locked
assertEq(rewardToken.balanceOf(potAddr), 9); // Dust locked
}

Recommended Mitigation

// After distribution
uint256 remainingBalance = i_token.balanceOf(address(this));
if (remainingBalance > 0) {
i_token.transfer(owner(), remainingBalance);
}

[L-2] ERC20 transfer/transferFrom Return Values Are Unchecked

Severity: Low
Category: Accounting Drift
Affected Code: src/Pot.sol:55, src/Pot.sol:64-66, src/ContestManager.sol:37

Vulnerability Details

transfer/transferFrom return values are ignored. With non-reverting "false-returning" ERC20 tokens, transfers silently no-op while bookkeeping still decrements.

Impact

  • Accounting drift between internal state and actual token balances

  • Potential fund loss with non-standard ERC20 implementations

Recommended Mitigation

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
using SafeERC20 for IERC20;
​
// Use safe versions
i_token.safeTransfer(recipient, amount);
i_token.safeTransferFrom(msg.sender, address(this), amount);

[L-3] No Rescue Path for Any Error Mode

Severity: Low
Category: Protocol Robustness
Affected Code: src/Pot.sol, src/ContestManager.sol

Vulnerability Details

Neither contract has emergency withdrawal or token recovery functions. All vulnerabilities above result in "tokens permanently locked" because no rescue mechanism exists.

Recommended Mitigation

function recoverTokens(IERC20 token, uint256 amount) external onlyOwner {
token.transfer(owner(), amount);
}
​
function recoverETH() external onlyOwner {
payable(owner()).transfer(address(this).balance);
}

Technical Reproduction

Prerequisites

forge install

Running Tests

forge test --match-path test/AuditPoC.t.sol -vvvv

Test Results

  • 9 of 10 PoC tests pass

  • H-2 test requires assertion correction for proper verification

  • All tests reproduce the described vulnerabilities

Conclusion

Critical Findings Summary

The core high-severity issues all stem from closePot()/claimCut() being stateless:

  1. No deadline on claims → H-3: Post-close double-spend

  2. No closed/terminal state → H-4: Re-callable drain

  3. Wrong redistribution denominator → H-1: Permanent fund lock

  4. Fee routed to msg.sender → H-2: Protocol revenue locked

Recommended Fix Priority

  1. Immediate: Implement H-1 through H-4 fixes (critical fund loss)

  2. High Priority: Implement M-1 through M-4 (config errors and edge cases)

  3. Standard: Implement L-1 through L-3 (operational robustness)

Overall Assessment

As written, every contest with an unclaimed player permanently locks user funds, and the protocol can never collect its fee. The fixes in [H-1]–[H-4] plus the input validation in [M-1]/[M-2]/[M-4] address the full set of vulnerabilities. No contract should be deployed without these fixes implemented.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-01] Owner Cut Stuck in `ContestManager`

## Description When `closeContest` function in the `ContestManager` contract is called, `pot` sends the owner's cut to the `ContestManager` itself, with no mechanism to withdraw these funds. ## Vulnerability Details: Relevant code - [Pot](https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/Pot.sol#L7) [ContestManager](https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/ContestManager.sol#L16-L26) The vulnerability stems from current ownership implementation between the `Pot` and `ContestManager` contracts, leading to funds being irretrievably locked in the `ContestManager` contract. 1. **Ownership Assignment**: When a `Pot` contract is created, it assigns `msg.sender` as its owner: ```solidity contract Pot is Ownable(msg.sender) { ... } ``` 2. **Contract Creation Context**: The `ContestManager` contract creates new `Pot` instances through its `createContest` function: ```solidity function createContest(...) public onlyOwner returns (address) { Pot pot = new Pot(players, rewards, token, totalRewards); ... } ``` In this context, `msg.sender` for the new `Pot` is the `ContestManager` contract itself, not the external owner who called `createContest`. 3. **Unintended Ownership**: As a result, the `ContestManager` becomes the owner of each `Pot` contract it creates, rather than the intended external owner. 4. **Fund Lock-up**: When `closeContest` is called (after the 90-day contest period), it triggers the `closePot` function: ```solidity function closeContest(address contest) public onlyOwner { Pot(contest).closePot(); } ``` The `closePot` function sends the owner's cut to its caller. Since the caller is `ContestManager`, these funds are sent to and locked within the `ContestManager` contract. 5. **Lack of Withdrawal Mechanism**: The `ContestManager` contract does not include any functionality to withdraw or redistribute these locked funds, rendering them permanently inaccessible. This ownership misalignment and the absence of a fund recovery mechanism result in a critical vulnerability where contest rewards become permanently trapped in the `ContestManager` contract. ## POC In existing test suite, add following test ```solidity function testOwnerCutStuckInContestManager() public mintAndApproveTokens { vm.startPrank(user); contest = ContestManager(conMan).createContest( players, rewards, IERC20(ERC20Mock(weth)), 100 ); ContestManager(conMan).fundContest(0); vm.stopPrank(); // Fast forward 91 days vm.warp(block.timestamp + 91 days); uint256 conManBalanceBefore = ERC20Mock(weth).balanceOf(conMan); console.log("contest manager balance before:", conManBalanceBefore); vm.prank(user); ContestManager(conMan).closeContest(contest); uint256 conManBalanceAfter = ERC20Mock(weth).balanceOf(conMan); // Assert that the ContestManager balance has increased (owner cut is stuck) assertGt(conManBalanceAfter, conManBalanceBefore); console.log("contest manager balance after:", conManBalanceAfter); } ``` run `forge test --mt testOwnerCutStuckInContestManager -vv` in the terminal and it will return following output: ```js [⠊] Compiling... [⠑] Compiling 1 files with Solc 0.8.20 [⠘] Solc 0.8.20 finished in 1.66s Compiler run successful! Ran 1 test for test/TestMyCut.t.sol:TestMyCut [PASS] testOwnerCutStuckInContestManager() (gas: 810988) Logs: User Address: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Contest Manager Address 1: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 Minting tokens to: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Approved tokens to: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 contest manager balance before: 0 contest manager balance after: 10 Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 10.51ms (1.31ms CPU time) ``` ## Impact Loss of funds for the protocol / owner ## Recommendations Add a claimERC20 function `ContestManager` to solve this issue. ```solidity function claimStuckedERC20(address tkn, address to, uint256 amount) external onlyOwner { // bytes4(keccak256(bytes('transfer(address,uint256)'))); (bool success, bytes memory data) = tkn.call(abi.encodeWithSelector(0xa9059cbb, to, amount)); require( success && (data.length == 0 || abi.decode(data, (bool))), 'ContestManager::safeTransfer: transfer failed' ); ```

Support

FAQs

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

Give us feedback!