Alchemix Transmuter
Alchemix
DeFiFoundrySolidity
16,653 OP
View results
Previous Next
Submission Details
Severity: high
Invalid

Not having the claiming of the assets in the report function can result on inaccurate totalAssets amount

kalogerone

Summary

According to the Yearn v3 docs, the _harvestAndReport function:

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.

However, this is not the case. The claimAndSwap function which claims the available WETH and reinvests the profit is a different function. Not having both the functions executed in the same call can create issues with the totalAssets of the vault as they are not accurate at all times.

Vulnerability Details

This is the same _harvestAndReport function throughout the 3 strategies:

StrategyOp.sol
​
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 function is called every time the Keepers call the report() function lying in the TokenizedStrategy.sol. This function is repsonsible to correctly update the totalAssets of the vault so the shares get minted/burnt correctly. As we can see the claimable amount is not accounted for. This means that this function should always get called in the same call exactly after claimAndSwap.

SrtategyOp.sol
​
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path)
external
onlyKeepers
{
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
​
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
​
uint256 balAfter = asset.balanceOf(address(this));
​
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
​
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
// TODO : we swap WETH to ALETH -> need to check that price is better than 1:1
// uint256 oraclePrice = 1e18 * 101 / 100;
require(minOut > _amount, "minOut too low");
​
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}

As we can see, calling claimAndSwap doesn't update the totalAssets of the vault.

Let's consider a scenario:

  1. Vault gets deployed, bob deposits 2 alETH, receives 2e18 shares and the vault has unexchanged = 2 alETH and claimable = 0 WETH.

  2. Time passes, now vault has unexchanged = 1 alETH and claimable = 1 WETH and report() gets called, so _harvestAndReport returns 1e18. Alice sees this transaction and it's always profitable for her to backrun and deposit her 1 alETH after the report() and get more shares than intended. While with 1 alETH should get 1/3 of the total vault shares, she will actually receive 1/2 of the total vault shares and she does that and gets 2e18 shares.

Impact

Having the claiming of the profits and the accounting of the totalAssets of the vault on different functions is very dangerous. The totalAssets can be wrongly reported and all the vault's convertToShares and convertToAssets functions will return inaccurate results.

Tools Used

Manual review

Recommendations

As per the yearn v3 docs, include the claiming and reinvesting of the tokens in the _harvestAndReport function.

Updates

Appeal created

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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