Becasue of devision before multiplication in LibFertilizer:remainingRecapitalization() less fertilizers is mintable than is needed to fully recapitalize the lost LP token value

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/LibFertilizer.sol#L182-L195

https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/C.sol#L188-L190

https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/beanstalk/barn/FertilizerFacet.sol#L64-L84

Summary

There is a division before multiplication at LibFertilizer:remainingRecapitalisation(), which will cause the protocol to enable the mint of less fertilizer than intended. This will result in less value recapitalized than lost and therefor loss of value for the holders of UNRIPE_LP tokens.

Vulnerability Details

When calling LibFertilizer::remainingRecapitalization() the amount of totalDollars this calculated that still need to be recapitalized:

     uint256 totalDollars = C.dollarPerUnripeLP().mul(C.unripeLP().totalSupply()).div(DECIMALS);

For this the function C.dollarPerUnripeLP is used invoking a division operation before the following multiplication, leading to precision loss:

     function dollarPerUnripeLP() internal pure returns (uint256) {
        return 1e12/UNRIPE_LP_PER_DOLLAR;
    }

POC

Add this file to test folder and run forge test -vv --mt test_precision_loss

// SPDX-License-Identifier: MIT
pragma solidity =0.7.6;
import {SafeMath} from "openzeppelin/utils/math/SafeMath.sol";
import "forge-std/Test.sol";
import "forge-std/console.sol";

contract TestPrecisionLoss is Test {
    using SafeMath for uint256;

    function setUp() public {}

    function test_precision_loss() external {
        uint256 UNRIPE_LP_TOTAL_SUPPLY = 96033202232306;
        uint128 DECIMALS = 1e6;
        uint256 UNRIPE_LP_PER_DOLLAR = 1884592; // 145_113_507_403_282 / 77_000_000
        uint256 dollarPerUnripeLP = 1e12 / UNRIPE_LP_TOTAL_SUPPLY;

        uint256 totalDollars1 = (1e12 / UNRIPE_LP_PER_DOLLAR)
            .mul(UNRIPE_LP_TOTAL_SUPPLY)
            .div(DECIMALS);
        console.log("totalDollars1: ", totalDollars1);
        uint256 totalDollars2 = (1e12 * UNRIPE_LP_TOTAL_SUPPLY)
            .div(UNRIPE_LP_PER_DOLLAR)
            .div(DECIMALS);
        console.log("totalDollars2: ", totalDollars2);
        console.log("the diff: ", totalDollars2 - totalDollars1);
    }
}

The output of the test is the following:

Running 1 test for test/testPrecisionLoss.sol:TestPrecisionLoss
[PASS] test_precision_loss() (gas: 5691)
Logs:
  totalDollars1:  50956945702101
  totalDollars2:  50957025304313
  the diff:  79602212

Impact

remainingRecapitalization() will always return lesser value than it should be. This will lead to less fertilizer available for mint and therefore less USD value that will get recapitalized for the UNRIPE_LP holders. Here is the related code in FertilizerFacet::mintFertilizer():

        uint128 remaining = uint128(LibFertilizer.remainingRecapitalization().div(1e6));// remaining <= 77_000_000 so downcasting is safe.
        require(fertilizerAmountOut <= remaining, "Fertilizer: Not enough remaining.");

Tools Used

Foundry

Recommendations

--uint256 totalDollars = C.dollarPerUnripeLP().mul(C.unripeLP().totalSupply()).div(DECIMALS);
++uint256 totalDollars = (1e12).mul(C.unripeLP().totalSupply()).div(C.unripeLPPerDollara()).div(DECIMALS);