1. harvestTrigger can revert

[smonicas]

Severity: Low				
Difficulty: Medium
Type: Data Validation			
Target: BaseStrategy.sol

Description
A function to know when to trigger in the strategy can revert. This can potentially block future calls or make them more expensive than necessary.

Before doing a call to harvest(), it is expected that an external system will check the result of harvestTrigger() to determinate if it makes sense to call it or not. This could be a script run off a desktop or cloud bot or via an integration with the Keep3r network:

    function harvestTrigger(uint256 callCost) public view virtual returns (bool) {
        StrategyParams memory params = vault.strategies(address(this));

        // Should not trigger if Strategy is not activated
        if (params.activation == 0) return false;

        // Should not trigger if we haven't waited long enough since previous harvest
        if (block.timestamp - params.lastReport < minReportDelay) return false;

        // Should trigger if hasn't been called in a while
        if (block.timestamp - params.lastReport >= maxReportDelay) return true;

        // If some amount is owed, pay it back
        // NOTE: Since debt is based on deposits, it makes sense to guard against large
        //       changes to the value from triggering a harvest directly through user
        //       behavior. This should ensure reasonable resistance to manipulation
        //       from user-initiated withdrawals as the outstanding debt fluctuates.
        uint256 outstanding = vault.debtOutstanding();
        if (outstanding > debtThreshold) return true;

        // Check for profits and losses
        uint256 total = estimatedTotalAssets();
        // Trigger if we have a loss to report
        if (total + debtThreshold < params.totalDebt) return true;

        uint256 profit = 0;
        if (total > params.totalDebt) profit = total - params.totalDebt; // We've earned a profit!

        // Otherwise, only trigger if it "makes sense" economically (gas cost
        // is <N% of value moved)
        uint256 credit = vault.creditAvailable();
        return (profitFactor * callCost < credit + profit);
    }

Figure 1: BaseStrategy.sol#L430-L462

However, if the callCost parameter is too large, the call can revert when computing the return value in the last line of the function.

Exploit scenario
Alice create a program that queries the strategy to call harvestTrigger() in order to know when to call harvest. Since she is willing to pay any price to call harvest, she uses a very large value as callCost. The transaction reverts, blocking any future calls to harvest.

Recommendation
Short term, make sure harvestTrigger will not revert, even for large callCost values.

Long term, use Echidna or Manticore to make sure important functions cannot revert unexpectedly.

[kitty-the-kat]

Acknowledged - This issue will be handled on the ops side and can be circumvented by sanity checking any input values to harvestTrigger. Won't solve this issue for now.