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
11 months ago

Appeal created

inallhonesty Lead Judge 11 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.

Give us feedback!