Insufficient validation of new Fertilizer IDs allow for a denial-of-service (DoS) attack on `SeasonFacet::gm` when above peg, once the last element in the FIFO is paid
A Fertilizer NFT can be interpreted as a bond without an expiration date which is to be repaid in Beans and includes interest (Humidity). This bond is placed in a FIFO list and intended to recapitalize the $77 million in liquidity stolen during the April 2022 exploit. One Fertilizer can be purchased for 1 USD worth of WETH: prior to BIP-38, this purchase was made using USDC.\\nEach fertilizer is identified by an Id that depends on `s.bpf`, indicating the cumulative amount of Beans paid per Fertilizer. This value increases each time `Sun::rewardToFertilizer` is called, invoked by `SeasonFacet::gm` if the Bean price is above peg. Therefore, Fertilizer IDs depend on `s.bpf` at the moment of minting, in addition to the amount of Beans to be paid.\\nThe FIFO list has following components:\\ns.fFirst: Fertilizer Id corresponding to the next Fertilizer to be paid.\\ns.fLast: The highest active Fertilizer Id which is the last Fertilizer to be paid.\\ns.nextFid: Mapping from Fertilizer Id to Fertilizer id, indicating the next element of a linked list. If an Id points to 0, then there is no next element.\\nMethods related to this FIFO list include: LibFertilizer::push: Add an element to the FIFO list. LibFertilizer::setNext: Given a fertilizer id, add a pointer to next element in the list LibFertilizer::getNext: Get next element in the list.\\nThe intended behaviour of this list is to add a new element to its end whenever a new fertilizer is minted with a new Id. Intermediate addition to the list was formerly allowed only by the Beanstalk DAO, but this functionality has since been deprecated in the current upgrade with the removal of `FertilizerFacet::addFertilizerOwner`.\\nConsequences of replacing BEAN:3CRV MetaPool with the BEAN:ETH Well: Before this upgrade, addition of 0 Fertilizer through `LibFertilizer::addFertilizer` was impossible due to the dependency on Curve in LibFertilizer::addUnderlying:\\n```\\n// Previous code\\n\\n    function addUnderlying(uint256 amount, uint256 minAmountOut) internal {\\n        //// rest of code\\n        C.bean().mint(\\n            address(this),\\n            newDepositedBeans.add(newDepositedLPBeans)\\n        );\\n\\n        // Add Liquidity\\n        uint256 newLP = C.curveZap().add_liquidity(\\n            C.CURVE_BEAN_METAPOOL, // where to add liquidity\\n            [\\n                newDepositedLPBeans, // BEANS to add\\n                0,\\n                amount, // USDC to add\\n                0\\n            ], // how much of each token to add\\n            minAmountOut // min lp ampount to receive\\n        ); // @audit-ok Does not admit depositing 0 --> https://etherscan.io/address/0x5F890841f657d90E081bAbdB532A05996Af79Fe6#code#L487\\n\\n        // Increment underlying balances of Unripe Tokens\\n        LibUnripe.incrementUnderlying(C.UNRIPE_BEAN, newDepositedBeans);\\n        LibUnripe.incrementUnderlying(C.UNRIPE_LP, newLP);\\n\\n        s.recapitalized = s.recapitalized.add(amount);\\n    }\\n```\\n\\nHowever, with the change of dependency involved in the Wells integration, this restriction no longer holds:\\n```\\n    function addUnderlying(uint256 usdAmount, uint256 minAmountOut) internal {\\n        AppStorage storage s = LibAppStorage.diamondStorage();\\n        // Calculate how many new Deposited Beans will be minted\\n        uint256 percentToFill = usdAmount.mul(C.precision()).div(\\n            remainingRecapitalization()\\n        );\\n        uint256 newDepositedBeans;\\n        if (C.unripeBean().totalSupply() > s.u[C.UNRIPE_BEAN].balanceOfUnderlying) {\\n            newDepositedBeans = (C.unripeBean().totalSupply()).sub(\\n                s.u[C.UNRIPE_BEAN].balanceOfUnderlying\\n            );\\n            newDepositedBeans = newDepositedBeans.mul(percentToFill).div(\\n                C.precision()\\n            );\\n        }\\n\\n        // Calculate how many Beans to add as LP\\n        uint256 newDepositedLPBeans = usdAmount.mul(C.exploitAddLPRatio()).div(\\n            DECIMALS\\n        );\\n\\n        // Mint the Deposited Beans to Beanstalk.\\n        C.bean().mint(\\n            address(this),\\n            newDepositedBeans\\n        );\\n\\n        // Mint the LP Beans to the Well to sync.\\n        C.bean().mint(\\n            address(C.BEAN_ETH_WELL),\\n            newDepositedLPBeans\\n        );\\n\\n        // @audit If nothing was previously deposited this function returns 0, IT DOES NOT REVERT\\n        uint256 newLP = IWell(C.BEAN_ETH_WELL).sync(\\n            address(this),\\n            minAmountOut\\n        );\\n\\n        // Increment underlying balances of Unripe Tokens\\n        LibUnripe.incrementUnderlying(C.UNRIPE_BEAN, newDepositedBeans);\\n        LibUnripe.incrementUnderlying(C.UNRIPE_LP, newLP);\\n\\n        s.recapitalized = s.recapitalized.add(usdAmount);\\n    }\\n```\\n\\nGiven that the new integration does not revert when attempting to add 0 Fertilizer, it is now possible to add a self-referential node to the end FIFO list, but only if this is the first Fertilizer NFT to be minted for the current season by twice calling `FertilizerFacet.mintFertilizer(0, 0, 0, mode)`. The validation performed to prevent duplicate ids is erroneously bypassed given the Fertilizer amount for the given Id remains zero.\\n```\\n    function push(uint128 id) internal {\\n        AppStorage storage s = LibAppStorage.diamondStorage();\\n        if (s.fFirst == 0) {\\n            // Queue is empty\\n            s.season.fertilizing = true;\\n            s.fLast = id;\\n            s.fFirst = id;\\n        } else if (id <= s.fFirst) {\\n            // Add to front of queue\\n            setNext(id, s.fFirst);\\n            s.fFirst = id;\\n        } else if (id >= s.fLast) { // @audit this block is entered twice\\n            // Add to back of queue\\n            setNext(s.fLast, id); // @audit the second time, a reference is added to the same id\\n            s.fLast = id;\\n        } else {\\n            // Add to middle of queue\\n            uint128 prev = s.fFirst;\\n            uint128 next = getNext(prev);\\n            // Search for proper place in line\\n            while (id > next) {\\n                prev = next;\\n                next = getNext(next);\\n            }\\n            setNext(prev, id);\\n            setNext(id, next);\\n        }\\n    }\\n```\\n\\nDespite first perhaps seeming harmless, this element can never be remove unless otherwise overridden:\\n```\\n    function pop() internal returns (bool) {\\n        AppStorage storage s = LibAppStorage.diamondStorage();\\n        uint128 first = s.fFirst;\\n        s.activeFertilizer = s.activeFertilizer.sub(getAmount(first)); // @audit getAmount(first) would return 0\\n        uint128 next = getNext(first);\\n        if (next == 0) { // @audit next != 0, therefore this conditional block is skipped\\n            // If all Unfertilized Beans have been fertilized, delete line.\\n            require(s.activeFertilizer == 0, "Still active fertilizer");\\n            s.fFirst = 0;\\n            s.fLast = 0;\\n            s.season.fertilizing = false;\\n            return false;\\n        }\\n        s.fFirst = getNext(first); // @audit this gets s.first again\\n        return true; // @audit always returns true for a self-referential node\\n    }\\n```\\n\\n`LibFertilizer::pop` is used in `Sun::rewardToFertilizer` which is called through `Sun::rewardBeans` when fertilizing. This function is called through `Sun::stepSun` if the current Bean price is above peg. By preventing the last element from being popped from the list, assuming this element is reached, an infinite loop occurs given that the `while` loop continues to execute, resulting in denial-of-service on `SeasonFacet::gm` when above peg.\\nThe most remarkable detail of this issue is that this state can be forced when above peg and having already been fully recapitalized. Given that it is not possible to mint additional Fertilizer with the associated Beans, this means that a DoS attack can be performed on `SeasonFacet::gm` once recapitalization is reached if the BEAN price is above peg.
Despite being a complex issue to explain, the solution is as simple as replacing `>` with `>=` in `LibFertilizer::addFertilizer` as below:\\n```\\n    function addFertilizer(\\n        uint128 season,\\n        uint256 fertilizerAmount,\\n        uint256 minLP\\n    ) internal returns (uint128 id) {\\n        AppStorage storage s = LibAppStorage.diamondStorage();\\n\\n        uint128 fertilizerAmount128 = fertilizerAmount.toUint128();\\n\\n        // Calculate Beans Per Fertilizer and add to total owed\\n        uint128 bpf = getBpf(season);\\n        s.unfertilizedIndex = s.unfertilizedIndex.add(\\n            fertilizerAmount.mul(bpf)\\n        );\\n        // Get id\\n        id = s.bpf.add(bpf);\\n        // Update Total and Season supply\\n        s.fertilizer[id] = s.fertilizer[id].add(fertilizerAmount128);\\n        s.activeFertilizer = s.activeFertilizer.add(fertilizerAmount);\\n        // Add underlying to Unripe Beans and Unripe LP\\n        addUnderlying(fertilizerAmount.mul(DECIMALS), minLP);\\n        // If not first time adding Fertilizer with this id, return\\n// Remove the line below\\n       if (s.fertilizer[id] > fertilizerAmount128) return id;\\n// Add the line below\\n       if (s.fertilizer[id] >= fertilizerAmount128) return id; // prevent infinite loop in `Sun::rewardToFertilizer` when attempting to add 0 Fertilizer, which could DoS `SeasonFacet::gm` when recapitalization is fulfilled\\n        // If first time, log end Beans Per Fertilizer and add to Season queue.\\n        push(id);\\n        emit SetFertilizer(id, bpf);\\n    }\\n```\\n
It is possible to perform a denial-of-service (DoS) attack on `SeasonFacet::gm` if the Bean price is above the peg, either once fully recapitalized or when reaching the last element of the Fertilizer FIFO list.\\nProof of Concept: This coded PoC can be run by:\\nCreating file `Beantalk/protocol/test/POCs/mint0Fertilizer.test.js`\\nNavigating to `Beantalk/protocol`\\nRunning `yarn test --grep "DOS last fertilizer payment through minting 0 fertilizers"`
```\\n// Previous code\\n\\n    function addUnderlying(uint256 amount, uint256 minAmountOut) internal {\\n        //// rest of code\\n        C.bean().mint(\\n            address(this),\\n            newDepositedBeans.add(newDepositedLPBeans)\\n        );\\n\\n        // Add Liquidity\\n        uint256 newLP = C.curveZap().add_liquidity(\\n            C.CURVE_BEAN_METAPOOL, // where to add liquidity\\n            [\\n                newDepositedLPBeans, // BEANS to add\\n                0,\\n                amount, // USDC to add\\n                0\\n            ], // how much of each token to add\\n            minAmountOut // min lp ampount to receive\\n        ); // @audit-ok Does not admit depositing 0 --> https://etherscan.io/address/0x5F890841f657d90E081bAbdB532A05996Af79Fe6#code#L487\\n\\n        // Increment underlying balances of Unripe Tokens\\n        LibUnripe.incrementUnderlying(C.UNRIPE_BEAN, newDepositedBeans);\\n        LibUnripe.incrementUnderlying(C.UNRIPE_LP, newLP);\\n\\n        s.recapitalized = s.recapitalized.add(amount);\\n    }\\n```\\n