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

_harvestAndReport returns incorrect value

Description

According to the docs StrategyOp::_harvestAndReport, StrategyArb::_harvestAndReport and StrategyMainnet::_harvestAndReport are crucial for accounting purposes and therefor should return an accurate value of all funds currently held by the strategy.
The issue arises since the sum in the return Value simply forgets to add uint256 claimable

Vulnerable Code:

StrategyOp::_harvestAndReport, StrategyArb::_harvestAndReport and StrategyMainnet::_harvestAndReport:

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

As you can see on the highlighted lines of code, claimable correctly gets fetched, but is than forgotten in the return statement of _totalAssets potentially critically harming relying functions with an incorrect return value.

Impact:

According to the Documentation the _harvestAndReport function is considered as THE source of truth for accounting purposes (quote: "A trusted and accurate account for the total amount of 'asset' the strategy currently holds including idle funds."), therefore the impact of returning an incomplete value should be considered High by default.

Likelihood: High
Impact: High

Severity: High

Tools Used:

Manual review.

Recommended Mitigation:

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;
+ _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance + claimable;
}
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.