Vanguard
First Flight #56
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Missing State Update for Fee Tracking

leoyeesir

Root + Impact

Description

The contract declares totalPenaltyFeesCollected as a public state variable (line 38) intended to track cumulative penalty fees collected by the protocol. However, in the _beforeSwap function (lines 174-177), when a penalty is applied and transferred, this variable is never incremented. The penalty fee is calculated, transferred to the owner, and an event is emitted—but the accounting state remains unchanged.

// TokenLaunchHook.sol:51
uint256 public totalPenaltyFeesCollected; // Declared but never updated
​
// TokenLaunchHook.sol:175
if (applyPenalty) {
​
feeOverride = uint24((phasePenaltyBps * 100));
// MISSING: totalPenaltyFeesCollected += penaltyFee;
}
​

Risk

Likelihood:

Medium - Every penalty transaction fails to update the tracking variable. Since penalties are core to the launch mechanism (Phase 1: 50%, Phase 2: 20%), this occurs whenever users exceed swap limits.

Impact:

Medium - Administrative and operational consequences:

  1. Broken transparency: Protocol cannot query total fees collected via totalPenaltyFeesCollected()

  2. Broken fee-sharing logic: Any future feature relying on this variable (e.g., staking rewards, fee distribution) will read 0

  3. Off-chain accounting burden: Must rely solely on event logs (PenaltyApplied) rather than on-chain state

  4. Integration failures: External contracts or analytics reading totalPenaltyFeesCollected receive incorrect data

Proof of Concept


Below code demo the lack of tracking totalfeecollected

function test_IgnoringPenaltyFeesPoC() public {
address attacker = address(0xBEEF);
vm.deal(attacker, 100 ether);
​
// First, do a tiny swap to trigger the initialLiquidity snapshot if it was 0
swapRouter.swap{value: 1 wei}(
key,
SwapParams({
zeroForOne: true,
amountSpecified: -1,
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
}),
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}),
ZERO_BYTES
);
​
// Now liquidity is snapshotted.
uint256 exactLimit = (hook.initialLiquidity() * hook.phase1LimitBps()) / 10000;
console.log("Exact Phase 1 Swap Limit (in liquidity units):", exactLimit);
​
// Attacker swaps 50 ETH (Violation!)
uint256 swapAmt = 50 ether;
​
uint256 feesCollectedBefore = hook.totalPenaltyFeesCollected();
console.log("Penalty fees collected before attack:", feesCollectedBefore);
​
vm.startPrank(attacker);
swapRouter.swap{value: swapAmt}(
key,
SwapParams({
zeroForOne: true,
amountSpecified: -int256(swapAmt),
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
}),
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}),
ZERO_BYTES
);
vm.stopPrank();
​
// 1. The trade MUST succeed (anti-bot doesn't block, it just taxes)
assertGt(token.balanceOf(attacker), 0, "Trade failed - anti-bot blocked instead of taxing");
​
// 2. The local penalty tracking should have incremented (BUT IT WON'T)
uint256 feesCollectedAfter = hook.totalPenaltyFeesCollected();
​
console.log("Bug Confirmed: Protocol recorded", feesCollectedAfter - feesCollectedBefore, "penalty fees");
assertEq(feesCollectedAfter, feesCollectedBefore, "Penalty fees were NOT recorded in state");
}

Recommended Mitigation

Increment totalPenaltyFeesCollected when penalty fees are transferred to maintain accurate on-chain accounting.

if (applyPenalty) {
uint256 penaltyFee = (uint256(-params.amountSpecified) * feeBps) / 10000;
tokenIn.transfer(owner, penaltyFee);
// FIX: Update tracking variable
totalPenaltyFeesCollected += penaltyFee;
emit PenaltyApplied(poolKey.toId(), msg.sender, penaltyFee, phase);
}
​
Updates

Lead Judging Commences

chaossr Lead Judge
17 days ago

Appeal created

chaossr Lead Judge 17 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!