Inadequate slippage control
The current slippage control mechanism checks a user's acceptable interest rate limit against the post-trade rate, which could result in trades proceeding at rates exceeding the user's defined limit.\\n```\\nFile: InterestRateCurve.sol\\n    function _getNetCashAmountsUnderlying(\\n        InterestRateParameters memory irParams,\\n        MarketParameters memory market,\\n        CashGroupParameters memory cashGroup,\\n        int256 totalCashUnderlying,\\n        int256 fCashToAccount,\\n        uint256 timeToMaturity\\n    ) private pure returns (int256 postFeeCashToAccount, int256 netUnderlyingToMarket, int256 cashToReserve) {\\n        uint256 utilization = getfCashUtilization(fCashToAccount, market.totalfCash, totalCashUnderlying);\\n        // Do not allow utilization to go above 100 on trading\\n        if (utilization > uint256(Constants.RATE_PRECISION)) return (0, 0, 0);\\n        uint256 preFeeInterestRate = getInterestRate(irParams, utilization);\\n\\n        int256 preFeeCashToAccount = fCashToAccount.divInRatePrecision(\\n            getfCashExchangeRate(preFeeInterestRate, timeToMaturity)\\n        ).neg();\\n\\n        uint256 postFeeInterestRate = getPostFeeInterestRate(irParams, preFeeInterestRate, fCashToAccount < 0);\\n        postFeeCashToAccount = fCashToAccount.divInRatePrecision(\\n            getfCashExchangeRate(postFeeInterestRate, timeToMaturity)\\n        ).neg();\\n```\\n\\nWhen executing a fCash trade, the interest rate is computed based on the utilization of the current market (Refer to Line 432). The `postFeeInterestRate` is then computed based on the `preFeeCashToAccount` and trading fee, and this rate will be used to derive the exchange rate needed to convert `fCashToAccount` to the net prime cash (postFeeCashToAccount).\\nNote that the interest rate used for the trade is `postFeeInterestRate`, and `postFeeCashToAccount` is the amount of cash credit or debit to an account.\\nIf there is any slippage control in place, the slippage should be checked against the `postFeeInterestRate` or `postFeeCashToAccount`. As such, there are two approaches to implementing slippage controls:\\n1st Approach - The current interest rate is `2%`. User sets their acceptable interest rate limit at 3% when the user submits the trade transaction. The user's tolerance is `1%`. From the time the trade is initiated to when it's executed, the rate (postFeeInterestRate) rises to 5%, the transaction should revert due to the increased slippage beyond the user's tolerance.\\n2nd Approach - If a user sets the minimum trade return of 1000 cash, but the return is only 900 cash (postFeeCashToAccount) when the trade is executed, the transaction should revert as it exceeded the user's slippage tolerance\\nNote: When users submit a trade transaction, the transaction is held in the mempool for a period of time before executing, and thus the market condition and interest rate might change during this period, and slippage control is used to protect users from these fluctuations.\\nHowever, within the codebase, it was observed that the slippage was not checked against the `postFeeInterestRate` or `postFeeCashToAccount`.\\n```\\nFile: InterestRateCurve.sol\\n        // returns the net cash amounts to apply to each of the three relevant balances.\\n        (\\n            int256 netUnderlyingToAccount,\\n            int256 netUnderlyingToMarket,\\n            int256 netUnderlyingToReserve\\n        ) = _getNetCashAmountsUnderlying(\\n            irParams,\\n            market,\\n            cashGroup,\\n            totalCashUnderlying,\\n            fCashToAccount,\\n            timeToMaturity\\n        );\\n..SNIP..\\n        {\\n            // Do not allow utilization to go above 100 on trading, calculate the utilization after\\n            // the trade has taken effect, meaning that fCash changes and cash changes are applied to\\n            // the market totals.\\n            market.totalfCash = market.totalfCash.subNoNeg(fCashToAccount);\\n            totalCashUnderlying = totalCashUnderlying.add(netUnderlyingToMarket);\\n\\n            uint256 utilization = getfCashUtilization(0, market.totalfCash, totalCashUnderlying);\\n            if (utilization > uint256(Constants.RATE_PRECISION)) return (0, 0);\\n\\n            uint256 newPreFeeImpliedRate = getInterestRate(irParams, utilization);\\n..SNIP..\\n            // Saves the preFeeInterestRate and fCash\\n            market.lastImpliedRate = newPreFeeImpliedRate;\\n        }\\n```\\n\\nAfter computing the net prime cash (postFeeCashToAccount == netUnderlyingToAccount) at Line 373 above, it updates the `market.totalfCash` and `totalCashUnderlying`. Line 395 computes the `utilization` after the trade happens, and uses the latest `utilization` to compute the new interest rate after the trade and save it within the `market.lastImpliedRate`\\n```\\nFile: TradingAction.sol\\n    function _executeLendBorrowTrade(\\n..SNIP..\\n        cashAmount = market.executeTrade(\\n            account,\\n            cashGroup,\\n            fCashAmount,\\n            market.maturity.sub(blockTime),\\n            marketIndex\\n        );\\n\\n        uint256 rateLimit = uint256(uint32(bytes4(trade << 104)));\\n        if (rateLimit != 0) {\\n            if (tradeType == TradeActionType.Borrow) {\\n                // Do not allow borrows over the rate limit\\n                require(market.lastImpliedRate <= rateLimit, "Trade failed, slippage");\\n            } else {\\n                // Do not allow lends under the rate limit\\n                require(market.lastImpliedRate >= rateLimit, "Trade failed, slippage");\\n            }\\n        }\\n    }\\n```\\n\\nThe trade is executed at Line 256 above. After the trade is executed, it will check for the slippage at Line 264-273 above.\\nLet $IR_1$ be the interest rate used during the trade (postFeeInterestRate), $IR_2$ be the interest rate after the trade (market.lastImpliedRate), and $IR_U$ be the user's acceptable interest rate limit (rateLimit).\\nBased on the current slippage control implementation, $IR_U$ is checked against $IR_2$. Since the purpose of having slippage control in DeFi trade is to protect users from unexpected and unfavorable price changes during the execution of a trade, $IR_1$ should be used instead.\\nAssume that at the time of executing a trade (TradeActionType.Borrow), $IR_1$ spikes up and exceeds $IR_U$. However, since the slippage control checks $IR_U$ against $IR_2$, which may have resettled to $IR_U$ or lower, the transaction proceeds despite exceeding the user's acceptable rate limit. So, the transaction succeeds without a revert.\\nThis issue will exacerbate when executing large trades relative to pool liquidity.
Consider updating the slippage control to compare the user's acceptable interest rate limit (rateLimit) against the interest rate used during the trade execution (postFeeInterestRate).
The existing slippage control does not provide the desired protection against unexpected interest rate fluctuations during the transaction. As a result, users might be borrowing at a higher cost or lending at a lower return than they intended, leading to losses.
```\\nFile: InterestRateCurve.sol\\n    function _getNetCashAmountsUnderlying(\\n        InterestRateParameters memory irParams,\\n        MarketParameters memory market,\\n        CashGroupParameters memory cashGroup,\\n        int256 totalCashUnderlying,\\n        int256 fCashToAccount,\\n        uint256 timeToMaturity\\n    ) private pure returns (int256 postFeeCashToAccount, int256 netUnderlyingToMarket, int256 cashToReserve) {\\n        uint256 utilization = getfCashUtilization(fCashToAccount, market.totalfCash, totalCashUnderlying);\\n        // Do not allow utilization to go above 100 on trading\\n        if (utilization > uint256(Constants.RATE_PRECISION)) return (0, 0, 0);\\n        uint256 preFeeInterestRate = getInterestRate(irParams, utilization);\\n\\n        int256 preFeeCashToAccount = fCashToAccount.divInRatePrecision(\\n            getfCashExchangeRate(preFeeInterestRate, timeToMaturity)\\n        ).neg();\\n\\n        uint256 postFeeInterestRate = getPostFeeInterestRate(irParams, preFeeInterestRate, fCashToAccount < 0);\\n        postFeeCashToAccount = fCashToAccount.divInRatePrecision(\\n            getfCashExchangeRate(postFeeInterestRate, timeToMaturity)\\n        ).neg();\\n```\\n