DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: low
Invalid

Identically across all strategy variants (StrategyArb.sol, StrategyMainnet.sol, StrategyOp.sol) due to their shared architecture and swap validation approach

Summary

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
// @FOUND Incorrect balance tracking point - balanceBefore is captured after claiming WETH,
// making it impossible to accurately track total value change from the swap
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
  1. The balance check is fundamentally flawed because it captures balanceBefore AFTER claiming WETH from the transmuter

  2. This means the initial balance snapshot doesn't include the pre-claim state

  3. The profitability comparison (balAfter - balBefore >= minOut) only tracks alETH balance changes

  4. The total value change calculation misses the WETH reduction from the swap

This creates a state transition vulnerability where:

  • Initial State: Strategy has X alETH, 0 WETH

  • Post-Claim: Strategy has X alETH, Y WETH

  • Post-Swap: Strategy has (X + Z) alETH, 0 WETH

Even if Z < Y in value terms (making the swap unprofitable), the check passes because it only sees the alETH increase of Z.

Vulnerability Details

In StrategyArb.claimAndSwap

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
// @FOUND The transmuter.claim() call changes the strategy's state by adding WETH before measuring balanceBefore
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
// @FOUND Insufficient profit validation - only checks alETH increase without accounting for WETH decrease
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

In StrategyMainnet.claimAndSwap

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
// @FOUND Same vulnerability - state change before balance check
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
require(_minOut > _amountClaim, "minOut too low");
router.exchange(routes[_routeNumber], swapParams[_routeNumber], _amountClaim, _minOut, pools[_routeNumber], address(this));
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

The strategies implement a flawed profitability check mechanism where:

  • WETH is claimed from transmuter before measuring initial balance

  • Only alETH balance changes are validated

  • WETH reduction from swaps is not factored into profitability calculations

Impact

  1. Value Extraction:

    • Keepers can execute unprofitable trades where WETH value lost > alETH value gained

    • Strategy's total value (WETH + alETH) can decrease while passing all checks

  2. Broken Core Invariant:

    • The contract's assumption that all swaps must be profitable is violated

    • This breaks the fundamental guarantee of value preservation

The issue is from incorrect state transition handling where the initial balance measurement happens after a significant state change (the claim operation), making it impossible to properly validate the total value change from the swap operation.

Recommendations

For the profitability validation issue across all strategy variants

// StrategyArb.sol, StrategyMainnet.sol, StrategyOp.sol
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
+ // @Mitigation - Capture total value before any state changes
+ uint256 totalValueBefore = balanceDeployed();
transmuter.claim(_amountClaim, address(this));
- uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
- uint256 balAfter = asset.balanceOf(address(this));
- require((balAfter - balBefore) >= _minOut, "Slippage too high");
+ // @Mitigation - Validate total value increased after swap
+ uint256 totalValueAfter = balanceDeployed();
+ require(totalValueAfter > totalValueBefore, "Trade not profitable");
+ require(totalValueAfter >= totalValueBefore + _minOut, "Minimum profit not met");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago

Appeal created

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID] Keepers can execute swaps that appear profitable in isolation but actually decrease total strategy value

Support

FAQs

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