Alchemix Transmuter

Summary

StrategyMainnet::claimAndSwap does not update the earned yield after a swap is made. As a result, when users exchange their vault shares for alETH tokens, they will receive an inaccurate amount of assets. This behavior will lead to a loss of funds for users who exchange their shares after the execution of claimAndSwap.

Vulnerability Details

Users can deposit their alETH into the strategy to earn yield on their tokens. When a user calls the deposit function, the strategy contract deposits these funds into the Transmuter contract. The strategy generates yield using the claimAndSwap function when the strategy can swap WETH to alETH at a premium. To achieve this, the strategy claims transmuted WETH, swaps it for alETH, and deposits the swapped alETH back into the Transmuter contract. Consequently, the strategy accumulates more alETH than the user originally deposited.

The issue is that the claimAndSwap function does not report this earned yield as profit.

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L92-L113

Yearn V3 strategies implement the ERC4626 standard, where the price of an asset and its shares is based on the _totalAssets variable (in this implementation). When the strategy earns yield, the profits should immediately be accounted for to ensure accurate calculations. This ensures users receive the expected and fair amount of assets or shares when interacting with the strategy. Yearn V3 strategies include a specific internal function, _harvestAndReport, designed to accurately reflect the total assets controlled by the strategy.

If _harvestAndReport is not invoked after the strategy earns yield, users cannot claim their rightful share of the profits.

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L172-L192

The claimAndSwap function fails to update _totalAssets when the strategy earns yield. As a result, calculations for assets and shares become inaccurate, leading to loss of funds for users. To resolve this, the functionality of _harvestAndReport should be changed so that whenever a report is called, the strategy performs the swap, accrues yield, and updates the _totalAssets variable.

For illustrative purposes, I provided simplified calculations, omitting AMM formulas and liquidity details to focus on the concept of the exploit.

1. A user deposits 100 alETH into the strategy. At the time, 1 WETH is worth 1 alETH.

2. After some time, the value of 1 WETH increases to 1.1 alETH.

3. The keeper calls the claimAndSwap function, which claims 1 WETH and swaps it for 1.1 alETH.

4. Now, the strategy holds 99 alETH (from the original deposit) + 1.1 alETH (from the swap), totaling 100.1 alETH.

5. The user decides to exchange all their vault shares for assets. They call the withdraw function and receive 100 alETH for all of their shares. However, the user initially had 100% of the shares and should have been entitled to withdraw all the strategy's assets (excluding fees and precision loss).

6. After the user's withdrawal, the keeper calls the report function to update the state of the contract. By this time, the user has already withdrawn their assets, resulting in the user losing the additional yield they earned by depositing into the strategy.

It is not the user's fault for calling withdraw or redeem, as these functions are expected to always return the most up-to-date asset values. Users should be able to call these functions at any time, and they should work correctly.

The keeper functionality is outside the scope of this audit and is not mentioned in the README. It is unclear how the keeper functionality will be implemented. It is possible that the keeper makes two separate calls: first to claimAndSwap and then to report. However, if the claimAndSwap functionality were integrated into _harvestAndReport, the issue would be avoided. In such a case, the keeper would only need to make a single call to the report function, ensuring all calculations are accurate and up-to-date.

For more information please take a look at report function in TokenizedStrategy contract. It gives more context on why any yield earning logic should be implemented in _harvestAndReport.

https://github.com/yearn/tokenized-strategy/blob/9ef68041bd034353d39941e487499d111c3d3901/src/TokenizedStrategy.sol#L1052-L1256

The most important thing is that the new _totalAssets from _harvestAndReport are used for share and asset calculations.

https://github.com/yearn/tokenized-strategy/blob/9ef68041bd034353d39941e487499d111c3d3901/src/TokenizedStrategy.sol#L1095-L1096

Impact

claimAndSwap fails to account for earned yield, which will result in users not receiving their fair share of the profits.

It is important to note that this report specifically addresses the vulnerability in the StrategyMainnet contract. While the claimAndSwap function is implemented in all three strategies, the root causes of the issue are different and one fix will not fix all of the three issues.

Tools Used

Manual review

Recommended Mitigation

One of the solutions to this issue is to integrate the claimAndSwap functionality into _harvestAndReport. Since swapping is the only source of yield IMPLEMENTED in this strategy, it should be performed within _harvestAndReport to accurately account for profits and ensure that the asset and share prices in the strategy remain correct.