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

minOut > _amountClaim doesn't guarantee profitability due to potential manipulation of balanceDeployed() calculation

Summary

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
// VULNERABILITY: Balance check only verifies asset token balance difference,
// not accounting for the claimed underlying tokens that are being swapped away.
// This allows value extraction since underlying tokens are counted in balanceDeployed()
// but their loss isn't factored into the profit calculation
_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 profitability check only looks at the difference in asset token balance (alETH), it ignores the value of the underlying tokens (WETH) being swapped away.

The balanceDeployed() function counts both tokens

function balanceDeployed() public view returns (uint256) {
// VULNERABILITY: Both underlying and asset tokens are counted at face value,
// allowing profitable-looking but value-extracting swaps to pass
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}

This creates a discrepancy between

  • How profit is checked during swaps (only asset balance difference)

  • How total value is tracked (both tokens counted)

A keeper could execute a swap that

  1. Claims 100 WETH

  2. Swaps for 101 alETH

  3. Appears profitable (+1 token)

  4. Actually loses value when both tokens should be counted equally

Vulnerability Details

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L71-L78

// In all the strategy implementations
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, ...) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
// ROOT CAUSE: The profit check only considers the asset (alETH) balance increase
// but ignores the value of underlying (WETH) being given up in the swap
_swapUnderlyingToAsset(_amountClaim, _minOut, ...);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
}

The issue affects all actors

  1. Keepers can execute technically profitable but economically losing swaps

  2. Depositors lose value through unfavorable swaps

  3. Strategy performance is compromised

For example

Initial State:
- Strategy has 100 WETH claimed from transmuter
- balanceDeployed() = 100 (counting WETH)
Keeper executes swap:
- Swaps 100 WETH for 101 alETH
- balBefore = 0 alETH
- balAfter = 101 alETH
- minOut check passes (101 > 100)
- But actual value could be less if WETH:alETH rate isn't 1:1
Final State:
- Strategy now has 101 alETH worth less than original 100 WETH
- balanceDeployed() = 101 but true value decreased
  1. The profit check in claimAndSwap only looks at asset balance increase

  2. balanceDeployed() counts both tokens at face value

function balanceDeployed() public view returns (uint256) {
// ROOT CAUSE: Both tokens counted equally without price consideration
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}

This affects all supported tokens (WETH/alETH) across all implementations, potentially leading to value loss through seemingly profitable but actually disadvantageous swaps.

Impact

// In the StrategyArb.sol, StrategyMainnet.sol, and StrategyOp.sol:
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, ...) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
// FLAW: Profit verification only checks asset token increase
// while ignoring the underlying token decrease
_swapUnderlyingToAsset(_amountClaim, _minOut, ...);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
}

The bug manifests because the strategy claims WETH from the transmuter:

  1. Swaps WETH for alETH

  2. Only verifies that received alETH > spent WETH in nominal terms

  3. Ignores actual value relationship between tokens

Value Extraction

function balanceDeployed() public view returns (uint256) {
// BUG: Face value counting enables value extraction
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}

Affected Functions

// All the strategies implement this flawed logic:
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, ...) internal {
require(minOut > _amount, "minOut too low"); // Insufficient check
// Swap execution varies by chain but vulnerability persists
}

Recommendations

// for StrategyArb.sol, StrategyMainnet.sol, and StrategyOp.sol:
+ // Add oracle interface for price verification
+ interface IPriceOracle {
+ function getPrice(address token) external view returns (uint256);
+ }
+ // Add oracle address and minimum profit threshold
+ contract Strategy... {
+ IPriceOracle public oracle;
+ uint256 public constant MIN_PROFIT_BPS = 50; // 0.5% minimum profit
+
+ constructor(...) {
+ // Initialize oracle address based on chain
+ oracle = IPriceOracle(CHAIN_SPECIFIC_ORACLE);
}
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, ...) external onlyKeepers {
+ // Track total value before swap
+ uint256 totalValueBefore = _calculateTotalValue();
transmuter.claim(_amountClaim, address(this));
- uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, ...);
- uint256 balAfter = asset.balanceOf(address(this));
- require((balAfter - balBefore) >= _minOut, "Slippage too high");
+ // Verify true profit using oracle prices
+ uint256 totalValueAfter = _calculateTotalValue();
+ uint256 profitBps = ((totalValueAfter - totalValueBefore) * 10000) / totalValueBefore;
+ require(profitBps >= MIN_PROFIT_BPS, "Insufficient profit");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
+ // Add accurate value calculation using oracle
+ function _calculateTotalValue() internal view returns (uint256) {
+ uint256 underlyingPrice = oracle.getPrice(address(underlying));
+ uint256 assetPrice = oracle.getPrice(address(asset));
+
+ return (underlying.balanceOf(address(this)) * underlyingPrice) +
+ (asset.balanceOf(address(this)) * assetPrice) +
+ (transmuter.getUnexchangedBalance(address(this)) * assetPrice);
}
- function balanceDeployed() public view returns (uint256) {
- return transmuter.getUnexchangedBalance(address(this)) +
- underlying.balanceOf(address(this)) +
- asset.balanceOf(address(this));
+ function balanceDeployed() public view returns (uint256) {
+ return _calculateTotalValue();
}
}
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.