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

The inaccurate asset accounting in `_harvestAndReport` results in flawed profit/loss calculations during reporting.

siisivan

Summary

The _harvestAndReport function is responsible for calculating the total assets held by the strategy. However, the current implementation only accounts for the unexchanged balance in the Transmuter, excluding the exchanged balance (assets that have already been converted from alETH to WETH). This incomplete calculation could lead to inaccurate profit/loss reporting .

Vulnerability Details

According to Yearn's Guide to Creating V3 "Tokenized Strategies", the purpose of the _harvestAndReport function is to “be called during every report. This function 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” (in this strategy, the underlying asset is alETH). In other words, _harvestAndReport needs to account for all assets held by the strategy.

In this strategy, when users deposit alETH into the strategy, the strategy deposits these alETH into the Transmuter, where they are gradually converted into WETH. Therefore, the total assets of the strategy should include the idle assets in the strategy (alETH), the unexchanged underlying assets in the strategy, and both the unexchanged assets (alETH) and exchanged assets (WETH) in the Transmuter. The amount of assets already exchanged into WETH in the Transmuter is returned by this function:https://github.com/alchemix-finance/v2-foundry/blob/8915ef7b1714c16f9e260a4ef41c5f254d5b7f58/src/TransmuterV2.sol#L377

However, the _harvestAndReport function fails to account for the exchanged balance (i.e., the portion of alETH already converted into WETH within the Transmuter). As a result, the total assets calculated by _harvestAndReport are incomplete, and any report triggered by the keeper through the report function may not accurately reflect the strategy's actual holdings.

StrategyMainnet:: _harvestAndReport:

/**
* @dev Internal function to harvest all rewards, redeploy any idle
* funds and return an accurate accounting of all funds currently
* held by the Strategy.
*
* This should do any needed harvesting, rewards selling, accrual,
* redepositing etc. to get the most accurate view of current assets.
*
* NOTE: All applicable assets including loose assets should be
* accounted for in this function.
*
* Care should be taken when relying on oracles or swap values rather
* than actual amounts as all Strategy profit/loss accounting will
* be done based on this returned value.
*
* This can still be called post a shutdown, a strategist can check
* `TokenizedStrategy.isShutdown()` to decide if funds should be
* redeployed or simply realize any profits/losses.
*
* @return _totalAssets A trusted and accurate account for the total
* amount of 'asset' the strategy currently holds including idle funds.
*/
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 (although we can restrict to only claim & swap in one tx)
uint256 underlyingBalance = underlying.balanceOf(address(this));
@> _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}

StrategyArb:: _harvestAndReport:

/**
* @dev Internal function to harvest all rewards, redeploy any idle
* funds and return an accurate accounting of all funds currently
* held by the Strategy.
*
* This should do any needed harvesting, rewards selling, accrual,
* redepositing etc. to get the most accurate view of current assets.
*
* NOTE: All applicable assets including loose assets should be
* accounted for in this function.
*
* Care should be taken when relying on oracles or swap values rather
* than actual amounts as all Strategy profit/loss accounting will
* be done based on this returned value.
*
* This can still be called post a shutdown, a strategist can check
* `TokenizedStrategy.isShutdown()` to decide if funds should be
* redeployed or simply realize any profits/losses.
*
* @return _totalAssets A trusted and accurate account for the total
* amount of 'asset' the strategy currently holds including idle funds.
*/
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;
}
​

StrategyOp:: _harvestAndReport:

/**
* @dev Internal function to harvest all rewards, redeploy any idle
* funds and return an accurate accounting of all funds currently
* held by the Strategy.
*
* This should do any needed harvesting, rewards selling, accrual,
* redepositing etc. to get the most accurate view of current assets.
*
* NOTE: All applicable assets including loose assets should be
* accounted for in this function.
*
* Care should be taken when relying on oracles or swap values rather
* than actual amounts as all Strategy profit/loss accounting will
* be done based on this returned value.
*
* This can still be called post a shutdown, a strategist can check
* `TokenizedStrategy.isShutdown()` to decide if funds should be
* redeployed or simply realize any profits/losses.
*
* @return _totalAssets A trusted and accurate account for the total
* amount of 'asset' the strategy currently holds including idle funds.
*/
function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
​
uint256 claimable = transmuter.getClaimableBalance(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;
}

This omission may result in errors in profit/loss calculations, potentially leading to mismanagement of strategy assets or inaccurate representation of returns.

Impact

  • The calculation of the total assets held by the strategy is inaccurate, affecting the calculation of unlocked shares. This leads to the inability to accurately operate the locking and unlocking mechanisms, thereby impacting the unlocking and distribution of profits. Locked shares become disconnected from the actual asset value, which affects fair profit distribution and the long-term sustainability of the strategy.

  • Inaccuracies in the calculation of performance fees and protocol fees, if not considering exchanged assets, may lead to incorrect fee computations, thereby affecting the overall profitability and distribution of fees.

  • Inaccurate total asset calculations impact the calculation of PPS (Price per Share), causing PPS to be either too high or too low. This affects all operations related to asset value, such as redemption, reinvestment, and more. Long-term PPS errors can accumulate, affecting risk assessment and user trust.

Tools Used

Manual

Recommendations

Include Exchanged Balance in Calculations:
Update the _harvestAndReport function to incorporate the exchanged balance (claimable) retrieved from the Transmuter. This exchanged balance should be converted to the equivalent amount of the underlying asset (asset, i.e., alETH) based on the current market price (potentially requiring the integration of an oracle). This ensures that all assets, including those already converted to WETH, are accurately calculated and consistently represented in terms of the underlying asset.

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.