DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Valid

The function _harvestAndReport() doesn't give the intended balance and can lead to pay undue fees from yearn

Summary

The balance returned by _harvestAndReport() is incorrect. Since this balance is used to calculate fees paid to Yearn, calling the report function can result in paying undue fees.

Vulnerability Details

The vulnerability is located in the following line from the StrategyOp file:

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

The function should implement the following specification from:

https://docs.yearn.fi/developers/v3/strategy_writing_guide


_harvestAndReport() :

Purpose:

  • Called during every report. This should harvest and sell any rewards, reinvest any proceeds, perform any position maintenance and return a full accounting of a trusted amount denominated in the underlying asset the strategy holds.

Parameters: NONE

Returns:

  • _totalAssets: A trusted and accurate account for the total amount of 'asset' the strategy currently holds including loose funds.


The _totalAssets is calculated using the following formula :

_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;

Since the asset and the underlying can become depegged (which is a revenue source for the strategy), asset.balanceOf(address(this)) is not always consistent with the other two terms. Note: there is an inversion between the notion of underlying and asset between the TokenStrategy file and the Strategy file.

Even if the strategy hasn't worked (and even if it was supposed to work, as claim and swap are commented out in the _harvestReport, we can have a difference between newTotalAssets and oldTotalAssets in the following function :

https://github.com/yearn/tokenized-strategy/blob/9ef68041bd034353d39941e487499d111c3d3901/flattened/FlatTokenizedStrategy.sol#L2413

As so, yearn protocol fees will be applied and the protocol alchemix will lose some funds.

Impact

Medium . There's some level of disruption to the protocol's functionality or availability

From the yearn specification :

"The returned value is used to account for all strategy profits, losses and fees so care should be taken when relying on oracle values, LP prices etc. that have the potential to be manipulated."

Additionally, there is loss of funds as the protocol will pay undue fees to Yearn

Likehood:

Medium . This might occur under specific conditions: Depeg of ALETH and WETH (expected by the protocol to execute the strategy).

Tools Used

Manual review

Recommendations

Add a conversion function call to the other two terms (asset variable is the underlying in terms of TokenStrategy file context):

_totalAssets = conversionAssetToUnderlying(unexchanged )+ asset.balanceOf(address(this))) + conversionAssetToUnderlying(underlyingBalance);

Updates

Appeal created

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

balanceDeployed() and _harvestAndReport() add WETH and alETH, but they have different prices

Support

FAQs

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