Summary
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));
}
The profitability check only considers the local balance change of the synthetic asset (alETH) during the swap, but ignores the value reduction from claiming the underlying asset (WETH). The balance comparison (balAfter - balBefore) >= _minOut is insufficient because:
balBefore is taken after claiming WETH
The check doesn't account for the total value change including the claimed WETH
A swap could pass this check while still reducing the total strategy value (balanceDeployed())
This allows transactions that satisfy minOut but still result in a net loss when considering the total value of the strategy (WETH + alETH positions).
Vulnerability Details
In StrategyArb.claimAndSwap
function claimAndSwap(uint256 amount, uint256 minOut, IVeloRouter.route[] path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balanceBefore = 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 root cause is a fundamental flaw in the accounting logic
The function claims WETH from the transmuter
Only then captures balanceBefore
Performs the swap
Checks if the swap produced minOut tokens
Deposits back to transmuter
This sequence allows value extraction because
The balanceBefore is captured after claiming WETH, masking the true starting value
The check (balAfter - balBefore) >= _minOut only verifies the local swap outcome
The total strategy value (balanceDeployed) can decrease while still passing this check
Impact
A keeper can execute trades that appear profitable locally but reduce total strategy value
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L122-L124
function balanceDeployed() public view returns (uint256) {
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}
Recommendations
// StrategyArb.sol, StrategyOp.sol, StrategyMainnet.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));
+ // @Mitigation - Track total strategy value before operation
+ uint256 totalBefore = balanceDeployed();
+
+ // Execute claim and swap
+ transmuter.claim(_amountClaim, address(this));
+ _swapUnderlyingToAsset(_amountClaim, _minOut, _path);
+
+ // @Mitigation - Verify total strategy value increased
+ uint256 totalAfter = balanceDeployed();
+ require(totalAfter > totalBefore, "Trade must be profitable");
+
+ // @Mitigation - Add minimum profitability threshold
+ uint256 minProfitBps = 50; // 0.5%
+ require(
+ totalAfter >= totalBefore + (totalBefore * minProfitBps / 10000),
+ "Insufficient profit margin"
+ );
+
+ transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
+ // @Mitigation - Add view function to calculate expected profit
+ function calculateExpectedProfit(
+ uint256 _amountClaim,
+ IVeloRouter.route[] calldata _path
+ ) external view returns (uint256 expectedProfit) {
+ // Simulate the swap and return expected profit
+ // This helps keepers validate trades before execution
+ }
