Shell Protocol - skodi's results

Lines of code

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

Vulnerability details

Impact

AMM's invariants are broken which might result in stale/unprofitable swaps

Proof of Concept

the function depositGivenInputAmount is used to preview amount of LP tokens using the function _reserveTokenSpecified

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

The function above calculates final_utility(Uf) by updating the balances Xf and Yf according to the specified amount which is reserve token here. A malicious user can deposit a specified amount so high such that updated Yf/Xf ratio is grater than maximum slope allowed MAX_M (0x5f5e1000000000000000000) breaking the invariant as there is no sanity check on balances implemented here. since the utility has changed by a great factor a swap from specified token X to Y results in a loss to the users

Tools Used

manual review

Recommended Mitigation Steps

consider adding _checkBalances function in _reserveTokenSpecified

+     _checkBalances(int256 Xf, int256 Yf)

such that it dosen't allow users to deposit amounts that change the balances ratio by a huge factor grater than MAX_M

Assessed type

Invalid Validation