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

_harvestAndReport function does not report the correct total amount of applicable assets

crunter

Summary

According to the documentation of the _harvestAndReport function, all applicable assets, including loose assets, should be accounted for and reported. However, the total amount of assets reported is incomplete as it is not taking into account all the possible applicable funds.

Vulnerability Details

In all strategies, _harvestAndReport function documentation states that

  • All applicable assets, including loose assets, should be accounted for in this function

Applicable assets can be understood as the total sum of locked and unlocked (loose) assets held by the strategy:

  • unlocked underlying - existent balance of underlying in the strategy contract

  • unlocked synthetic - existent balance of synthetic in the strategy contract + unexchanged amount in transmuter

  • locked synthetic/underlying - amount deposited in the transmuter and generating yield

so that

  • _totalAssets = unlocked underlying + unlocked synthetic + locked synthetic/underlying

However, _harvestAndReport is only taking into account a partial amount of the applicable assets:

StrategyMainnet.sol
StrategyOp.sol
StrategyArb.sol
​
function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
// @audit - locked assets
@> uint256 claimable = transmuter.getClaimableBalance(address(this));
​
if (claimable > 0) {
// transmuter.claim(claimable, address(this));
// @audit - design choice: function does not claim available funds
}
​
// 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));
​
// @audit - The function is not taking into account the locked funds
@> _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}

This omission may result in unexpected behavior from processes that depend on the returned data.

Impact

Impact: Medium

Likelihood: High

Tools Used

Manual Review

Recommendations

It is recommended to include the locked balance in the reported assets count for the Mainnet, Optimism and Arbitrum strategies.

StrategyMainnet.sol
StrategyOp.sol
StrategyArb.sol
​
​
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;
+ _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance + claimable;
}
Updates

Appeal created

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