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

The function _harvestAndReport() is not implemented correctly and it will cause the loss of fees due to the protocol

Summary

Protocol and performances fees will never be collected because of incorrect implementation of the _harvestAndReport function, notably the harverst part.

Vulnerability Details

The vulnerability is in the following method :

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

The function must implement the following specification from :

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


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.


As the claim and swap functions are commented, when the function report will be called( https://github.com/yearn/tokenized-strategy/blob/9ef68041bd034353d39941e487499d111c3d3901/flattened/FlatTokenizedStrategy.sol#L2413)

the function harverstReport will returns a newTotalAssets amount equal to oldTotalAssets so the performance fee and protocol could never been accessed due to the following line :

if (newTotalAssets > oldTotalAssets) {

As the performance and protocol fees are only collected when the report function is called, theses fees will never be collected.

Impact

High. There's a severe disruption of protocol functionality or availability. Also a loss of funds for the protocol as the protocol fees are not retrieved as intended when the report function is called.

Likehood

High. The function will always not work as intended.

Tools Used

Manual

Recommendations

Uncomment the lines about swap and claim in the harverstReport function :

function \_harvestAndReport()\
internal\
override\
returns (uint256 \_totalAssets)\
{
uint256 claimable = transmuter.getClaimableBalance(address(this));
if (claimable > 0) {
transmuter.claim(claimable, address(this));
}
// NOTE : we can do this in harvest or can do seperately in tend
if (underlying.balanceOf(address(this)) > 0) {
_swapUnderlyingToAsset(underlying.balanceOf(address(this)));
}
uint256 unexchanged = transmuter.getUnexchangedBalance(address(this));
// NOTE : possible some dormant WETH that isn't swapped yet
uint256 underlyingBalance = underlying.balanceOf(address(this));
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
Updates

Appeal created

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

Incorrect accounting in `_harvestAndReport` claimable should be included

Support

FAQs

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