code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-shell/blob/main/src/proteus/EvolvingProteus.sol#L586-L594 https://github.com/code-423n4/2023-08-shell/blob/main/src/proteus/EvolvingProteus.sol#L658 https://github.com/code-423n4/2023-08-shell/blob/main/src/proteus/EvolvingProteus.sol#L441-L448

Vulnerability details

Impact

The EvolvingProteus._reserveTokenSpecified function is called to calculate the changed amount of the LP token supply based on the proportional change of the utility of the pool.

Firstly the initial utility and final utility are calculated as shown below:

    ui = _getUtility(xi, yi);
    uf = _getUtility(xf, yf);

Then the proportional change of the utility is used to calculate the final LP token supply amount as shown below:

    uint256 result = Math.mulDiv(uint256(uf), uint256(si), uint256(ui));
    require(result < INT_MAX);
    int256 sf = int256(result);

The issue here is the computed sf (final LP token supply amount) is not checked against the MIN_BALANCE value. Since _reserveTokenSpecified is called by both EvolvingProteus.depositGivenInputAmount and EvolvingProteus.withdrawGivenOutputAmount functions, the sf amount can decrease from the initial LP token supply amount of si during reserve token withdrawals.

This could prompt the sf < MIN_BALANCE condition to occur. If this condition occurs the transaction should revert as it is done in the EvolvingProteus._getUtilityFinalLp function. But the transaction will not revert in the _reserveTokenSpecified function execution since there is no conditional check to verify sf >= MIN_BALANCE.

Hence the withdrawGivenOutputAmount transaction will execute successfully without revert even if the key invariant sf >= MIN_BALANCE is broken. Hence this breaks the expected behaviour of the protocol.

Proof of Concept

        ui = _getUtility(xi, yi);
        uf = _getUtility(xf, yf);

        uint256 result = Math.mulDiv(uint256(uf), uint256(si), uint256(ui));
        require(result < INT_MAX);   
        int256 sf = int256(result);

        // apply fee to the computed amount
        computedAmount = _applyFeeByRounding(sf - si, feeDirection);

https://github.com/code-423n4/2023-08-shell/blob/main/src/proteus/EvolvingProteus.sol#L586-L594

        require(sf >= MIN_BALANCE);

https://github.com/code-423n4/2023-08-shell/blob/main/src/proteus/EvolvingProteus.sol#L658

        int256 result = _reserveTokenSpecified(
            withdrawnToken,
            -int256(withdrawnAmount),
            FEE_UP,
            int256(totalSupply),
            int256(xBalance),
            int256(yBalance)
        );

https://github.com/code-423n4/2023-08-shell/blob/main/src/proteus/EvolvingProteus.sol#L441-L448

Tools Used

VSCode and Manual Review

Recommended Mitigation Steps

Hence it is recommended to add the conditional check to verify sf >= MIN_BALANCE in the _reserveTokenSpecified function after the sf value is calculated as shown below. If the invariant for MIN_BALANCE is broken then the transaction should revert.

    uint256 result = Math.mulDiv(uint256(uf), uint256(si), uint256(ui));
    require(result < INT_MAX);
    int256 sf = int256(result);

    require(sf >= MIN_BALANCE);