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

Individual checks appear sound but fail to guarantee the overall system invariant of minimum output enforcement.

Summary

In claimAndSwap() function of StrategyOp.sol, the minOut enforcement mechanism across all strategy implementations.

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
// BUG: minOut check in _swapUnderlyingToAsset can be bypassed since it doesn't account for the total balance change
_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));
}
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
// BUG: This minOut check is insufficient as it doesn't account for the actual received amount
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
  1. The strategy implements two separate minOut checks that don't properly coordinate:

    • One in _swapUnderlyingToAsset() that compares minOut to input amount

    • Another after the swap comparing balance differences

  2. Neither check accounts for the total state change in the strategy's position

  3. The balance checks only look at asset.balanceOf() instead of the full balanceDeployed() calculation

This creates a scenario where:

  • A keeper could provide a minOut value that passes the first check (minOut > _amount)

  • Execute a swap that results in fewer tokens than promised

  • The second check using balanceOf() would pass due to not accounting for the full position

  • The final deposit to transmuter could fail or revert, leaving the strategy with less value than the minOut guarantee

Vulnerability Details

StrategyMainnet.claimAndSwap()

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
// BUG: The balance check sequence allows for manipulation of the minOut guarantee
// 1. Claims WETH from transmuter
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");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

The minOut enforcement fails because:

  1. The balance checks only consider asset.balanceOf() instead of the complete strategy position

  2. The transmuter deposit happens after the slippage check

  3. The strategy assumes the balance difference directly correlates to swap output

Impact across different implementations:

StrategyArb.claimAndSwap()

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
// BUG: Same vulnerability pattern across all implementations
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");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

This affects all supported blockchains (Optimism, Ethereum, Arbitrum), the:

  • Token flows between WETH and alETH

  • Keepers who can execute these swaps

  • Depositors whose share value could be impacted by manipulated swaps

Bcause:

  1. It exists in the core swap logic used across all implementations

  2. It affects the primary value flow of the strategy (WETH ↔ alETH)

  3. It could be exploited by keepers to extract value through manipulated swaps

Impact

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
// BUG: Balance checks don't capture full position state
// 1. Claims WETH but doesn't track it in balance check
transmuter.claim(_amountClaim, address(this));
// 2. Only tracks alETH balance, missing WETH position
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
// 3. Slippage check happens before final deposit
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
// 4. Final deposit could fail after slippage check passes
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
  1. The balance checks ignore the WETH position

  2. Slippage protection occurs before final position settlement

  3. The transmuter deposit happens after all safety checks

Value Extraction:

  • Keepers can manipulate swaps to extract value while passing checks

  • The strategy's accounting can become incorrect after failed deposits

Asset Loss Scenarios:

balanceDeployed() public view returns (uint256) {
// This total balance calculation can differ from what slippage checks verify
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}

Cross-Chain Impact:

  • Affects all implementations (Optimism, Ethereum, Arbitrum)

  • Each DEX integration (Velodrome, Curve, Ramses) is vulnerable

Protocol Integration Risk:

interface ITransmuter {
// Failed deposits after slippage checks can leave funds stranded
function deposit(uint256 _amount, address _owner) external;
function claim(uint256 _amount, address _owner) external;
}

Recommendations

// StrategyOp.sol, StrategyMainnet.sol, StrategyArb.sol
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
- 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");
- transmuter.deposit(asset.balanceOf(address(this)), address(this));
+ // Track full strategy position before operations
+ uint256 positionBefore = balanceDeployed();
+
+ // Execute operations in a single transaction
+ transmuter.claim(_amountClaim, address(this));
+ _swapUnderlyingToAsset(_amountClaim, _minOut, _path);
+
+ // Verify swap output before deposit
+ uint256 newAssets = asset.balanceOf(address(this));
+ require(newAssets >= _minOut, "Insufficient swap output");
+
+ // Complete position change
+ transmuter.deposit(newAssets, address(this));
+
+ // Verify total position change meets requirements
+ uint256 positionAfter = balanceDeployed();
+ require(positionAfter >= positionBefore + _minOut, "Position change below minimum");
}
// Add position validation helper
+ function validatePositionChange(uint256 before, uint256 after, uint256 minChange) internal pure {
+ require(after >= before, "Position decreased");
+ require(after - before >= minChange, "Insufficient position increase");
+ }
Updates

Lead Judging Commences

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