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

The _harvestAndReport() function does not work as intended

jovemjeune

Summary

Beside returning the _totalAssets,_harvestAndReport() method of strategy contracts aims harvesting all rewards and redeploying any idle funds. However it does not neither claim claimable funds from Transmuter nor redeploy idle funds. Additionaly calling _harvestAndReport() method from the strategy contracts is impossible since it is an internal function and called by no function inside the strategy contracts.

Vulnerability Details

This function is first defined inside BaseStrategy without any implementation and internal function call and by natspec it is specified that this function is designated for harvesting all rewards, redeploy any idle funds and return an accurate accounting of all funds currently held by the Strategy. However in strategy contracts this function is implemented as following :

//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;
}
//StrategyMainnet.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 (although we can restrict to only claim & swap in one tx)
uint256 underlyingBalance = underlying.balanceOf(address(this));
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
//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;
}
​

In none of them it claims claimable funds or redeploy idle funds. Additionaly since in this function is declared as internal and no function inside the contracts call it. It is not callable by any role or eoa address using the strategy contracts.

Impact

The _harvestAndReport() function will be uncallable however it will still increase the contract deployment cost in case the issue is not fixed.

Tools Used

Manual Analysis

Recommendations

The issue can be solved by completely implementing and calling this function by another function inside the contracts. In case there is indeed a donation attack risk exists in this function as it is written in the known issues section following method can be used inside the protocol which is suggested by openzeppelin : virtual offset.

Updates

Lead Judging Commences

inallhonesty Lead Judge
8 months ago

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.