Summary
While the code checks that _minOut > _amountClaim, this doesn't guarantee that the total strategy balance increases after the swap. The balanceDeployed() function includes:
Unexpended balance
Underlying balance
Asset balance
The current checks fail to account for the complete balance impact across all these components, allowing transactions that appear profitable in isolation but actually decrease the total strategy value.
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
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));
}
There exists a path where balanceAfter < balanceBefore despite all the existing checks passing.
Because the code makes an incorrect assumption that checking individual balances is sufficient to guarantee overall profitability, without considering the holistic impact on the strategy's total value.
Vulnerability Details
In StrategyArb.claimAndSwap
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 routeNumber) external onlyKeepers {
uint256 balanceBefore = balanceDeployed();
transmuter.claim(_amountClaim, address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balanceAfter = balanceDeployed();
require((balAfter - balBefore) >= _minOut, "Slippage too high");
}
In StrategyMainnet.claimAndSwap
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
router.exchange(routes[_routeNumber], swapParams[_routeNumber], _amountClaim, _minOut, pools[_routeNumber], address(this));
}
In StrategyOp._swapUnderlyingToAsset
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
require(minOut > _amount, "minOut too low");
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
In how profitability is calculated and checked, the strategies track three balance components:
Unexchanged balance (in transmuter)
Underlying token balance (WETH)
Asset balance (alETH)
The current implementations make an incorrect assumption that ensuring minOut > amount for swaps guarantees profitability. However, this fails to account for the complete state transition:
When transmuter.claim() is called, it reduces unexchanged balance and increases underlying balance
When swap occurs, it reduces underlying balance and increases asset balance
The final deposit to transmuter converts asset balance back to unexchanged balance
Because the checks only verify individual steps rather than the complete state transition's impact on total value.
Impact
Keepers can execute swaps that appear profitable in isolation but actually decrease total strategy value
Recommendations
// StrategyArb.sol
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 routeNumber) external onlyKeepers {
+ // @Mitigation - Track total value before any state changes
+ uint256 totalValueBefore = balanceDeployed();
transmuter.claim(_amountClaim, address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
- uint256 balanceAfter = balanceDeployed();
- require((balAfter - balBefore) >= _minOut, "Slippage too high");
+ // @Mitigation - Ensure total strategy value increases
+ uint256 totalValueAfter = balanceDeployed();
+ require(totalValueAfter > totalValueBefore, "Trade must be profitable");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
+ // @Mitigation - Add route validation
+ require(_routeNumber < nRoutes, "Invalid route");
+
+ uint256 totalBefore = balanceDeployed();
transmuter.claim(_amountClaim, address(this));
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
)
+
+ uint256 totalAfter = balanceDeployed();
+ require(totalAfter > totalBefore, "Must increase total value");
}
// StrategyOp.sol
+ // @Mitigation - Add minimum profit threshold
+ uint256 public constant MIN_PROFIT_BPS = 50; // 0.5%
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
- require(minOut > _amount, "minOut too low");
+ // @Mitigation - Enforce minimum profit margin
+ uint256 minProfit = (_amount * (10000 + MIN_PROFIT_BPS)) / 10000;
+ require(minOut >= minProfit, "Insufficient profit margin");
IVeloRouter(router).swapExactTokensForTokens(
_amount,
minOut,
_path,
address(this),
block.timestamp
);
}
