Shell Protocol - T1MOH's results

Lines of code

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

Vulnerability details

Impact

_checkBalances() is performed to ensure the balances of X and Y tokens are above MIN_BALANCE, and to ensure that Y/X ratio in [MIN_M; MAX_M) boundary. This check is performed in both swaps, depositGivenOutputAmount(), withdrawGivenInputAmount(). However there is no such a check in depositGivenInputAmount() and withdrawGivenOutputAmount() That is how implemented checks can be bypassed.

Proof of Concept

Main issue is that _checkBalances() is not invoked in _reserveTokenSpecified(). _reserveTokenSpecified() is used in calculations of depositGivenInputAmount() and withdrawGivenOutputAmount()

This test returns required amount of LP to withdraw whole token X reserve, paste to EvolvingProteusProperties.t.sol

    function testPOC_withdrawGivenOutputAmount() public {
        uint256 result = DUT.withdrawGivenOutputAmount(3000e18, 1000e18, 1000 ether, 3000e18, SpecifiedToken.X);
        console.log(result);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add this check:

    function _reserveTokenSpecified(
        SpecifiedToken specifiedToken,
        int256 specifiedAmount,
        bool feeDirection,
        int256 si,
        int256 xi,
        int256 yi
    ) internal view returns (int256 computedAmount) {
        ...

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

+       _checkBalances(xf, yf);
    }

Assessed type

Invalid Validation