code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L156-L171 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/mixins/VeloSolidMixin.sol#L32

Vulnerability details

Impact

The swap steps that are specified within the reaper strategy's ReaperStrategyGranarySupplyOnly::setHarvestSteps call are not sanitized as having a valid veloSwapPath defined, thereby permitting a swap step to be defined without a path to execute it and thus causing all ReaperStrategyGranarySupplyOnly::_harvestCore invocations to fail until a swap has been defined due to an arithmetic underflow.

This can be taken advantage of when a ReaperBaseStrategyv4::harvest transaction has been submitted but not executed yet to cause a DoS deliberately or it can result from misuse or incorrect coordination between transactors.

Proof of Concept

To illustrate the issue, the following contract has been devised:

contract InexistentSwapPath {
    /**
     * Simplified Assumptions:
     *
     * - Contract is at least `ADMIN` of `ReaperStrategyGranarySupplyOnly`
     */
    function demonstrate(IStrategy strategy, IERC20 swappableToken) external {
        // Flaw: Setup a swap step w/o a swap path
        address[2][] memory steps = new address[2](1);
        address[2] memory pair = [swappableToken, address(strategy.gWant())];
        steps.push(pair);
        strategy.setHarvestSteps(steps);

        // Step: Execute a harvest call 
        try strategy.harvest() {
            revert("Unreachable");
        } catch (bytes memory e) {
            revert("ISSUE: Harvest Failed");
        }
    }
}

Tools Used

Manual review.

Recommended Mitigation Steps

The ReaperStrategyGranarySupplyOnly::setHarvestSteps function should mandate that a valid veloSwapPath has already been defined for the swap steps being introduced to avoid unavailability of the protocol.