Alchemix Transmuter

Summary

In the _harvestAndReport functions the reported _totalAssets is given by
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance; which includes even the alETH and WETH directly held by the Strategy contract.

This _totalAssets is used by the report function to determine profit/loss for the strategy.
Which means the strategy considers every alETH or WETH directly sent to the contract to be rewards and include them in the _totalAssets calculation.

The problem is underlyingBalance is already considered as rewards but is still unwithdrawable/un-swappable i.e. locked forever.

Vulnerability Details

alETH is automatically converted to rewards by the TokenizedStrategy but for WETH we need a swap function which is missing in the Strategy.

The only conversion of WETH to ETH happens in the claimAndSwap function, but this function can only swap what is already in the transmuter contact and not those held directly by the Strategy contract.

It simply checks for to balance but only swap the WETH amount claimed from the transmuter. Which means they are un-swappable through the claimAndSwap function and needs a separate function(or modify this function to include underlyingBalance).

Now coming to _harvestAndReport function;

as we can see from the function that underlyingBalance is included, but since there is no function to swap it to alETH, it is locked in the contract forever, and must not be considered as rewards.

The commented out code actually tried to do the swap, which could have prevented this issue. Either the developer forgot to add a new swap function where the keeper can directly swap dormant WETH held in the contract or he forgot to remove underlyingBalance from the _totalAssets calculation.

Since this return value _totalAssets is used to calculate profit/loss.
Now an attacker can directly send some WETH to the contract and when report() is called it will update a false/unwithdrawable amount as profits which will affect the share to asset ratio and give unfair profits to early withdrawers and lock late withdrawers' assets.

Example(simplified):

• share to asset is 1:1 i.e. yalETH : alETH, with Bob and Alice as depositors with 50 shares each

• total shares = 100, total assets = 100

• Bob directly sends 10 WETH to the contract

• when report is called new totalAssets = 110, because of directly sent WETH

• then Strategy assumes 10 tokens as profits

• share to asset ratio is now 1:1.1

• Bob redeems all his shares amd gets 55 alETH back

• Alice tries to withdraw but can only do so upto 45 alETH and the remaining 5 tokens of alice are stuck in the contract forever.

• 10 WETH directly sent in the contract is also stuck forever.

Please note this is just a simple example which excludes the locking period, fees,etc. and other accounting made by the TokenizedStrategy.

Impact

Likelyhood: Low, since Bob(in the example) will lose tokens, but this could also be anyone.

Impact: High, because it is permanent, and it affects all the share holders(some get more, some lose).

Overall severity: Medium

Tools Used

manual

Recommendations

Introduce a new function:

the exact implementation will be different for the three Strategy contracts as they use different routers, but should look something like this in general.

With this function in place, even if Bob directly sends WETH then it can be considered as free rewards and Alice would not be hurt.

OR

We could abandon all WETH sent to the contract by removing the underlyingBalance from the _totalAsset calculations:
In StrategyMainnet, StrategyOp and StrategyArb contracts, the _harvestAndReport function should be changed to:

but this also means that any WETH held by Strategy will be locked and cannot be considered for rewards.