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

`_totalAssests` returned from `_harvestAndReport()` is incorrect

Summary

Vulnerability Details

_harvestAndReport()has following comment

* @return _totalAssets A trusted and accurate account for the total
* amount of 'asset' the strategy currently holds including idle funds.
*/

here it is getting quite confusing, cause contract considering `alETH` as ASSET and WETHas UNDERLYING.

But in function implementation its adding all

if i go though code comment as i mentioned above, its intend to include all assets(all type of funds i.e both alETH & WETH) here

_totalAssets here includeing

  • Unexchanged Amount from Transmuter

  • alETH balnce of contract

  • WETH balance of contract

But here it forgot to include claimable balance from transmutercause according to Transmuter Docs

  1. When a user deposits synthetic tokens into the Transmuter, the tokens are added to their UnexchangedBalance.

  2. Over time, as collateral becomes available, the Transmuter converts the synthetic tokens to the underlying asset. This process reduces the UnexchangedBalance and increases the ClaimableBalance.

So its possible that a significant amount of Claimable amount will be left behind.

Possible solution is that

  1. totalAssets should also add claimable to itself as it defined inside _harvestAndReport()

  2. _harvestAndReport() should always called after claimAndSwap()

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;
}

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

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

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

impact

Tools Used

Manual review

Recommendations

Possible solution is that

  1. totalAssets should also add claimable to itself as it defined inside _harvestAndReport()

Updates

Appeal created

inallhonesty Lead Judge 8 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.