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

The claim operation reduces the unexchanged balance

Summary

In StrategyArb.sol profitability check mechanism of the claimAndSwap function.

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
// @audit-issue Profitability check bypass: balanceBefore only captures asset balance, not total value
// This allows value extraction since unexchanged balance reduction isn't factored into profitability check
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 vulnerability is from an incorrect accounting model in the profitability verification. The function only checks the delta of the asset balance (alETH) but fails to account for the reduction in unexchanged balance from the transmuter.claim() operation.

Technical Flow:

  1. Initial State:

    • Unexchanged Balance: X

    • Asset Balance: Y

    • Total Value: X + Y

  2. After transmuter.claim():

    • Unexchanged Balance: (X - _amountClaim)

    • Underlying Balance: _amountClaim

    • Asset Balance: Y

  3. After swap:

    • Unexchanged Balance: (X - _amountClaim)

    • Asset Balance: Y + minOut

    • Total Value: (X - _amountClaim) + (Y + minOut)

The check require((balAfter - balBefore) >= _minOut) only verifies that asset balance increased by minOut, but doesn't ensure that: (X - _amountClaim) + (Y + minOut) >= X + Y

This allows a keeper to extract value by claiming large amounts and swapping them for marginally more than the minimum required increase in asset balance, while actually reducing the total value of the strategy.

Vulnerability Details

The bug exists across all three strategy implementations (StrategyArb.sol, StrategyOp.sol, and StrategyMainnet.sol) in their claimAndSwap functions.

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
// @audit-issue The profitability check is fundamentally flawed:
// 1. Claims WETH from transmuter reducing unexchanged balance
// 2. Only checks asset (alETH) balance increase
// 3. Allows value extraction since total value (unexchanged + underlying + asset) can decrease
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 same issue exists in StrategyMainnet.sol and StrategyOp.sol with their respective router implementations.

The bug pipe from an incorrect economic assumption in the profitability check. The strategies track three balances:

  1. Unexchanged balance (in transmuter)

  2. Underlying balance (WETH)

  3. Asset balance (alETH)

The total value is calculated in balanceDeployed()

function balanceDeployed() public view returns (uint256) {
// @audit-issue This shows the true economic value that should be checked in claimAndSwap
return transmuter.getUnexchangedBalance(address(this)) + underlying.balanceOf(address(this)) + asset.balanceOf(address(this));
}

Impact

A keeper can execute a value-extracting trade by:

  1. Claiming a large amount (_amountClaim) from transmuter

  2. Swapping for slightly more than _amountClaim

  3. While the asset balance increases by minOut, the total strategy value decreases

Example:

  • Initial State: 100 unexchanged + 0 underlying + 0 asset = 100 total

  • Claim 100: 0 unexchanged + 100 underlying + 0 asset = 100 total

  • Swap 100 for 101: 0 unexchanged + 0 underlying + 101 asset = 101 total

  • While asset increased by 1, if the true market rate was 1:1.05, this represents a loss of value

Because the transmuter.claim() operation's impact on unexchanged balance is not factored into the profitability verification, allowing economically unfavorable trades that appear profitable when only checking asset balance changes.

Recommendations

For StrategyArb.sol, StrategyOp.sol, and StrategyMainnet.sol

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path) external onlyKeepers {
+ // @audit-fix Track total value before operation to ensure true profitability
+ uint256 totalBefore = 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");
+ // @audit-fix Ensure total strategy value increases after swap
+ uint256 totalAfter = balanceDeployed();
+ require(totalAfter > totalBefore, "Trade must be profitable");
+ require(totalAfter >= totalBefore + _minOut, "Minimum profit not met");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
+ // @audit-fix Add minimum profit threshold that can be adjusted by management
+ uint256 public minProfitBps = 50; // 0.5% minimum profit
+
+ function setMinProfitBps(uint256 _minProfitBps) external onlyManagement {
+ require(_minProfitBps > 0, "Profit threshold too low");
+ require(_minProfitBps <= 1000, "Profit threshold too high");
+ minProfitBps = _minProfitBps;
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge
6 months ago

Appeal created

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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