Alchemix Transmuter
Alchemix
DeFiFoundrySolidity
16,653 OP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Keeper can manipulate swap paths to perform unintended swaps

setstacklist

In the StrategyOp and StrategyArb contracts, the claimAndSwap function allows a keeper to specify the swap path ( _path ) for swapping WETH (the underlying token) to alETH (the asset). The keeper provides this path when calling the function:

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 _swapUnderlyingToAsset function executes the swap based on the keeper-provided path:

function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
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);
}

Because the keeper controls the swap path, they could potentially provide a malicious path that swaps WETH into an unintended or less valuable token rather than alETH. This could cause the strategy to hold tokens that are not the intended asset, potentially leading to losses or requiring manual intervention to recover funds.

The risk arises because there is no check to ensure that the output token of the swap is indeed alETH. The only safeguard is the balance check of asset before and after the swap, but if the swap results in receiving a different token, this check would fail, and the transaction would revert. However, if the keeper can manipulate the swap in a way that passes the balance check (e.g., if a malicious token incorrectly reports balances), they could cause
undesired behavior.

In contrast, the StrategyMainnet contract mitigates this risk by storing predefined swap routes in contract storage, which are added via the addRoute function by management. The claimAndSwap function in StrategyMainnet uses a route number to reference these stored routes, preventing arbitrary paths from being used:

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));
}

By not allowing arbitrary swap paths, StrategyMainnet reduces the risk of keeper manipulation. The lack of similar restrictions in StrategyOp and StrategyArb presents a potential vulnerability where a malicious keeper could exploit the swap functionality.

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

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyOp.sol#L79

Updates

Appeal created

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