Shell Protocol - mert_eren's results

Lines of code

https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L563-L595

Vulnerability details

Impact

The _checkBalance function within the EvolvingProteus.sol contract serves the purpose of assessing whether the contract will remain stable after balance adjustments. This function is particularly relevant during balance changes for the contract.

    function _checkBalances(int256 x, int256 y) private pure {
        if (x < MIN_BALANCE || y < MIN_BALANCE) revert BalanceError(x,y);
        int128 finalBalanceRatio = y.divi(x);
        if (finalBalanceRatio < MIN_M) revert BoundaryError(x,y);
        else if (MAX_M <= finalBalanceRatio) revert BoundaryError(x,y);
    }

The inputs x and y denote the updated balances of the Pool's X and Y tokens after a transaction. This code confirms that both token balances must exceed the MIN_BALANCE value. Additionally, the ratio y/x should fall within the range defined by MIN_M and MAX_M.

    function _reserveTokenSpecified(
        SpecifiedToken specifiedToken,
        int256 specifiedAmount,
        bool feeDirection,
        int256 si,
        int256 xi,
        int256 yi
    ) internal view returns (int256 computedAmount) {
        int256 xf;
        int256 yf;
        int256 ui;
        int256 uf;
        {
            // calculating the final price points considering the fee
            if (specifiedToken == SpecifiedToken.X) {
                xf = xi + _applyFeeByRounding(specifiedAmount, feeDirection);
                yf = yi;
            } else {
                yf = yi + _applyFeeByRounding(specifiedAmount, feeDirection);
                xf = xi;
            }
        }


        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);
    }

    function _swap(
        bool feeDirection,
        int256 specifiedAmount,
        int256 xi,
        int256 yi,
        SpecifiedToken specifiedToken
    ) internal view returns (int256 computedAmount) {
        int256 roundedSpecifiedAmount;
        // calculating the amount considering the fee
        {
            roundedSpecifiedAmount = _applyFeeByRounding(
                specifiedAmount,
                feeDirection
            );
        }


        int256 xf;
        int256 yf;
        // calculate final price points after the swap
        {


            int256 utility = _getUtility(xi, yi);


            if (specifiedToken == SpecifiedToken.X) {
                int256 fixedPoint = xi + roundedSpecifiedAmount;
                (xf, yf) = _findFinalPoint(
                    fixedPoint,
                    utility,
                    _getPointGivenXandUtility
                );
            } else {
                int256 fixedPoint = yi + roundedSpecifiedAmount;
                (xf, yf) = _findFinalPoint(
                    fixedPoint,
                    utility,
                    _getPointGivenYandUtility
                );
            }
        }
        
        // balance checks with consideration the computed amount
        if (specifiedToken == SpecifiedToken.X) {
            computedAmount = _applyFeeByRounding(yf - yi, feeDirection);
            _checkBalances(xi + specifiedAmount, yi + computedAmount);
        } else {
            computedAmount = _applyFeeByRounding(xf - xi, feeDirection);
            _checkBalances(xi + computedAmount, yi + specifiedAmount);
        }
    }

    function _lpTokenSpecified(
        SpecifiedToken specifiedToken,
        int256 specifiedAmount,
        bool feeDirection,
        int256 si,
        int256 xi,
        int256 yi
    ) internal view returns (int256 computedAmount) {
        // get final utility considering the fee
        int256 uf = _getUtilityFinalLp(
            si,
            si + _applyFeeByRounding(specifiedAmount, feeDirection),
            xi,
            yi
        );


        // get final price points
        int256 xf;
        int256 yf;
        if (specifiedToken == SpecifiedToken.X)
            (xf, yf) = _findFinalPoint(
                yi,
                uf,
                _getPointGivenYandUtility
            );
        else
            (xf, yf) = _findFinalPoint(
                xi,
                uf,
                _getPointGivenXandUtility
            );


        // balance checks with consideration the computed amount
        if (specifiedToken == SpecifiedToken.X) {
            computedAmount = _applyFeeByRounding(xf - xi, feeDirection);
            _checkBalances(xi + computedAmount, yf);
        } else {
            computedAmount = _applyFeeByRounding(yf - yi, feeDirection);
            _checkBalances(xf, yi + computedAmount);
        }
    }

The _swap and _lpTokenSpecified functions utilize the _checkBalance function to verify new balances. In contrast, the _reserveTokenSpecified function omits this verification step, thereby leaving the functions using _reserveTokenSpecified vulnerable to potential balance instability. This flaw exposes the pool's balance to risks. Two functions rely on _reserveTokenSpecified:

    function depositGivenInputAmount(
        uint256 xBalance,
        uint256 yBalance,
        uint256 totalSupply,
        uint256 depositedAmount,
        SpecifiedToken depositedToken
    ) external view returns (uint256 mintedAmount) {
        // deposit amount validations against the current balance
        require(
            depositedAmount < INT_MAX &&
                xBalance < INT_MAX &&
                yBalance < INT_MAX &&
                totalSupply < INT_MAX
        );


        int256 result = _reserveTokenSpecified(
            depositedToken,
            int256(depositedAmount),
            FEE_DOWN,
            int256(totalSupply),
            int256(xBalance),
            int256(yBalance)
        );


        // amount cannot be less than 0
        require(result > 0);
        mintedAmount = uint256(result);
    }

    function withdrawGivenOutputAmount(
        uint256 xBalance,
        uint256 yBalance,
        uint256 totalSupply,
        uint256 withdrawnAmount,
        SpecifiedToken withdrawnToken
    ) external view returns (uint256 burnedAmount) {
        // withdraw amount validations against the current balance
        require(
            withdrawnAmount < INT_MAX &&
                xBalance < INT_MAX &&
                yBalance < INT_MAX &&
                totalSupply < INT_MAX
        );


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


        // amount cannot be less than 0
        require(result < 0);
        burnedAmount = uint256(-result);
    }

Both of these functions lack mechanisms to ensure balanced and healthy conditions after _reserveTokenSpecified too. This oversight renders them susceptible to potential manipulations affecting the pool's stability.

Proof of Concept

To demonstrate this vulnerability, I utilized the withdrawGivenOutputAmount function. The code below exemplifies this issue:

    function testDefault() public{

        DUT.withdrawGivenOutputAmount(10**18,10**18,10**18,10**18,SpecifiedToken.X);
    }

This function means that there is 1018 x and y tokens and user want to withdraw 1018 x token. If this function operates without reverting, a user could potentially withdraw all x tokens from the pool, as indicated by the withdrawAmount variable within the function. In this instance, the contract would allow this withdrawal despite violating the minimum balance requirement. This test underscores the vulnerability within the withdrawGivenOutputAmount function.

It's worth noting that other functions, such as those utilizing _swap or _lpTokenSpecified, would appropriately revert due to the balance checks enforced by _checkBalances.

Tools Used

Manuel review

Recommended Mitigation Steps

Use _checkBalances in _reserveTokenSpecified.

Assessed type

Invalid Validation