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

Locked WETH in strategies can't be swapped and will lead to inaccurate calculations of total assets and fee distributions during the harvest and report call.

rajko_z

Summary

Keepers have the ability to call a function to harvest and record all profits accrued or losses incurred within the strategy. This function is utilized in the TokenizedStrategy implementation contract on Yearn Finance:

https://github.com/yearn/tokenized-strategy/blob/master/src/TokenizedStrategy.sol#L1081

function report()
external
nonReentrant
onlyKeepers
returns (uint256 profit, uint256 loss)
{
// Cache storage pointer since its used repeatedly.
StrategyData storage S = _strategyStorage();
​
// Tell the strategy to report the real total assets it has.
// It should do all reward selling and redepositing now and
// account for deployed and loose `asset` so we can accurately
// account for all funds including those potentially airdropped
// and then have any profits immediately locked.
@=> uint256 newTotalAssets = IBaseStrategy(address(this))
.harvestAndReport();
​
....
}
​

During this call, specific code in the BaseStrategy will be executed to retrieve the total assets currently held:

function harvestAndReport() external virtual onlySelf returns (uint256) {
@=> return _harvestAndReport();
}

All three strategies override the _harvestAndReport call with the same logic.

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

function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
...
uint256 unexchanged = transmuter.getUnexchangedBalance(address(this));
// NOTE : possible some dormant WETH that isn't swapped yet (although we can restrict to only claim & swap in one tx)
@=> uint256 underlyingBalance = underlying.balanceOf(address(this));
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}

The main purpose of this function is to return an accurate count of alETH held by the strategy along with its idle tokens, enabling the report call within the strategy to accurately calculate new fees and assets. This function also includes the balance of WETH that has not yet been swapped, as noted in the comment. However, this dormant WETH won't be swappable since the only place where WETH is swapped is during the claimAndSwap call, which takes WETH from the Transumer contract, not directly from the strategy contract itself.

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
@=> transmuter.claim(_amountClaim, address(this));
...
}

This will lead to inaccurate totalAssets reported by the _harvestAndReport() function because it includes the WETH balance that can't be swapped for alETH. This miscalculation will be used by the report function, leading to consequences like:

  • Virtually increased profit and minting shares for fee recipients for assets not actually held by the strategy.

  • Invalid totalAssets updates that do not reflect the real, available amount of alETH.

function report()
external
nonReentrant
onlyKeepers
returns (uint256 profit, uint256 loss)
{
...
​
uint256 newTotalAssets = IBaseStrategy(address(this))
.harvestAndReport();
​
uint256 oldTotalAssets = _totalAssets(S);
// Calculate profit/loss.
if (newTotalAssets > oldTotalAssets) {
// We have a profit.
unchecked {
profit = newTotalAssets - oldTotalAssets;
}
​
// We need to get the equivalent amount of shares
// at the current PPS before any minting or burning.
@=> sharesToLock = _convertToShares(S, profit, Math.Rounding.Down);
​
...
​
// Mint the protocol fees to the recipient.
@=> _mint(S, protocolFeesRecipient, protocolFeeShares);
...
​
// Update the new total assets value.
@=> S.totalAssets = newTotalAssets;
S.lastReport = uint96(block.timestamp);
​
// Emit event with info
emit Reported(
profit,
loss,
protocolFees, // Protocol fees
totalFees - protocolFees // Performance Fees
);
}

Impact

Likelihood: Medium
For this to happen, there must be some WETH directly held by the strategy. Although WETH is not meant to be directly held by the strategy, as it would be immediately sold during the claimAndSwap, it is possible to donate some WETH directly to this contract. This would then be locked and erroneously included in the accounting during the harvest call.


Impact: Medium
New fee shares will be minted for assets not held by the strategy, which will dilute the price of the share. Also, totalAssets won't represent the real assets held by the strategy.

Tools Used

Manual review.

Recommendations

When calculating totalAssets during the harvest and report call, two options are available:

  1. Swap all WETH held by the strategy to alETH during the call.

  2. Exclude WETH from the totalAssets calculation and implement a separate method that will swap WETH directly held by the contract.

The first option is more gas-consuming since it requires a swap to be executed on every report from the keeper. Therefore, the second approach is recommended. This new method is not expected to be called frequently, only when there are some locked underlying tokens.

function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
...
- uint256 underlyingBalance = underlying.balanceOf(address(this));
- _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
+ _totalAssets = unexchanged + asset.balanceOf(address(this));
}
+ function swapUnderlying(
+ uint256 _minOut,
+ uint256 _routeNumber
+ ) external onlyKeepers {
+ uint256 balBefore = asset.balanceOf(address(this));
+ router.exchange(
+ routes[_routeNumber],
+ swapParams[_routeNumber],
+ underlying.balanceOf(address(this)),
+ _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));
+ }
Updates

Appeal created

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Dormant WETH in the contract will never be swapped back to alETH

rajko_z Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Dormant WETH is not properly treated

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.