Smart contract unable to operate due to lack of token funds
Description
Brief/Intro
Permit2 is approved the wrong asset which leads to loss of funds or failing swaps while later using the 0x Settler.
Vulnerability Details
First off, the intention for the Permit2 approvals by the Alchemix strategy contracts is to approve the deposit/withdrawal asset (USDC, WETH) to the Permit2 contract.
For the EulerWETHStrategy for example, this asset is WETH, also for the PeapodsETH strategy this asset is WETH, not _peapodsEth as it currently does.
contractPeapodsETHStrategyisMYTStrategy{ IERC4626 publicimmutable vault; WETH publicimmutable weth;// @audit has 1 issue with wrong asset being approved to permit2constructor(address_myt,StrategyParamsmemory_params,address_peapodsEth,address_weth,address_permit2Address) // @audit wrong args@> MYTStrategy(_myt, _params, _permit2Address, _peapodsEth){ vault =IERC4626(_peapodsEth); weth =WETH(_weth);}...}
As we can see above, we will approve _peapodsEth to the Permit2 contract instead of WETH.
These contracts below also wrongly approve the ERC4626 vault shares instead of the asset being allocated or will be later deallocated and swapped by the 0x Settler.
These are:
TokeAutoUSDStrategy
TokeAutoEthStrategy
PeapodsUSDCStrategy
Only the asset being deposited into the third party vaults (Euler, Peapods, Aave) are to be approved to Permit2. Not the vault shares itself which opens way for theft of the vault shares from the Alchemix strategy contracts.
Some references from the Alchemix protocol for this:
The allocator who is whitelisted to allocate/deallocate from the vault (and its corresponding strategies) fetches the 0x swap calldata from their rest api
But we do not trust this calldata blindly because the strategy was not part of this quote process so it might contain invalid assets or amounts
So the strategy calls the deployed library's verify call which is a simplified version of 0x's own internal verification again to ensure we only allow the strategies assets to be swapped, in one direction and with matching allocation amounts. Otherwise the 0x settler executes any quote in the name of the strategy contract that's valid.
Impact Details
There are two distinct impacts:
The 0x Settler swaps in the TokeAutoUSDStrategy, PeapodsUSDCStrategy, TokeAutoEthStrategy, and PeapodsETHStrategy strategies will fail
The Permit2 address will be approved for the wrong asset class and it will be possible to use the 0x Settler to swap out vault shares (e.g _peapodsEth and _peapodsUSDC) from the strategy contract which is not what the Alchemix team wants for these strategy contracts
Follow the same implementation as done in EulerWETHStrategy for example where we pass in WETH to the MYTStrategy constructor:
or if the intent is to approve the vault shares to permit2 and not the vault asset, then maintain that for all the strategies in-scope otherwise it would be possible to steal/swap out the other asset via permit2 in 0x settler swaps or the swaps completely fail because of wrong token approval in the constructor.
constructor(address _myt, StrategyParams memory _params, address _permit2Address, address _receiptToken) Ownable(_params.owner) {
...
permit2Address = _permit2Address;
// IERC20 vaultAsset = IERC20(address(MYT.asset()));
// vaultAsset.approve(permit2Address, type(uint256).max);
@> IERC20 receiptTokenContract = IERC20(receiptToken);
@> receiptTokenContract.approve(permit2Address, type(uint256).max);
// TODO add the strategy to the perpetual gauge in an authenticated manner
// TODO perhap take initial snapshot now to set up start block
}