CVE-2024-24746: Loop with Unreachable Exit Condition ('Infinite Loop') (CWE-835)
value: Loop with Unreachable Exit Condition ('Infinite Loop') vulnerability in Apache NimBLE. 

Specially crafted GATT operation can cause infinite loop in GATT server leading to denial of service in Bluetooth stack or device.

This issue affects Apache NimBLE: through 1.6.0.
Users are recommended to upgrade to version 1.7.0, which fixes the issue.
CWE description: The product contains an iteration or loop with an exit condition that cannot be reached, i.e., an infinite loop.
Published: 2024-04-06T12:15Z
Repository: https://github.com/apache/mynewt-nimble
Language: C
Files changed: nimble/host/src/ble_hs_conn.c
Commit date: 2024-02-14 21:27:16 +0100
Commit message: nimble/host: Fix disconnect on host connection timeout

We don't need to have double loop and lock-unlock host lock when
issuing disconnect. ble_gap_terminate_with_conn() can be used
to disconnect and it can be called with already provided conn object
under host lock.

Diff: diff --git a/nimble/host/src/ble_hs_conn.c b/nimble/host/src/ble_hs_conn.c
index 9b7bdbb3..57ba16af 100644
--- a/nimble/host/src/ble_hs_conn.c
+++ b/nimble/host/src/ble_hs_conn.c
@@ -475,90 +475,77 @@ ble_hs_conn_timer(void)
 
     return BLE_HS_FOREVER;
 #endif
 
     struct ble_hs_conn *conn;
-    ble_npl_time_t now;
-    int32_t next_exp_in;
+    ble_npl_time_t now = ble_npl_time_get();
+    int32_t next_exp_in = BLE_HS_FOREVER;
+    int32_t next_exp_in_new;
+    bool next_exp_in_updated;
     int32_t time_diff;
-    uint16_t conn_handle;
 
-    for (;;) {
-        conn_handle = BLE_HS_CONN_HANDLE_NONE;
-        next_exp_in = BLE_HS_FOREVER;
-        now = ble_npl_time_get();
+    ble_hs_lock();
 
-        ble_hs_lock();
-
-        /* This loop performs one of two tasks:
-         * 1. Determine if any connections need to be terminated due to timeout.
-         *    If so, break out of the loop and terminate the connection.  This
-         *    function will need to be executed again.
-         * 2. Otherwise, determine when the next timeout will occur.
-         */
-        SLIST_FOREACH(conn, &ble_hs_conns, bhc_next) {
-            if (!(conn->bhc_flags & BLE_HS_CONN_F_TERMINATING)) {
+    /* This loop performs one of two tasks:
+     * 1. Determine if any connections need to be terminated due to timeout. If
+     *    so connection is disconnected.
+     * 2. Otherwise, determine when the next timeout will occur.
+     */
+    SLIST_FOREACH(conn, &ble_hs_conns, bhc_next) {
+        if (!(conn->bhc_flags & BLE_HS_CONN_F_TERMINATING)) {
+            next_exp_in_updated = false;
 
 #if MYNEWT_VAL(BLE_L2CAP_RX_FRAG_TIMEOUT) != 0
-                /* Check each connection's rx fragment timer.  If too much time
-                 * passes after a partial packet is received, the connection is
-                 * terminated.
-                 */
-                if (conn->bhc_rx_chan != NULL) {
-                    time_diff = conn->bhc_rx_timeout - now;
-
-                    if (time_diff <= 0) {
-                        /* ACL reassembly has timed out.  Remember the connection
-                         * handle so it can be terminated after the mutex is
-                         * unlocked.
-                         */
-                        conn_handle = conn->bhc_handle;
-                        break;
-                    }
-
-                    /* Determine if this connection is the soonest to time out. */
-                    if (time_diff < next_exp_in) {
-                        next_exp_in = time_diff;
-                    }
-                }
-#endif
+            /* Check each connection's rx fragment timer.  If too much time
+             * passes after a partial packet is received, the connection is
+             * terminated.
+             */
+            if (conn->bhc_rx_chan != NULL) {
+                time_diff = conn->bhc_rx_timeout - now;
 
-#if BLE_HS_ATT_SVR_QUEUED_WRITE_TMO
-                /* Check each connection's rx queued write timer.  If too much
-                 * time passes after a prep write is received, the queue is
-                 * cleared.
-                 */
-                time_diff = ble_att_svr_ticks_until_tmo(&conn->bhc_att_svr, now);
                 if (time_diff <= 0) {
-                    /* ACL reassembly has timed out.  Remember the connection
-                     * handle so it can be terminated after the mutex is
-                     * unlocked.
-                     */
-                    conn_handle = conn->bhc_handle;
-                    break;
+                    /* ACL reassembly has timed out.*/
+                    ble_gap_terminate_with_conn(conn, BLE_ERR_REM_USER_CONN_TERM);
+                    continue;
                 }
 
                 /* Determine if this connection is the soonest to time out. */
                 if (time_diff < next_exp_in) {
-                    next_exp_in = time_diff;
+                    next_exp_in_new = time_diff;
+                    next_exp_in_updated = true;
                 }
+            }
 #endif
+
+#if BLE_HS_ATT_SVR_QUEUED_WRITE_TMO
+            /* Check each connection's rx queued write timer.  If too much
+             * time passes after a prep write is received, the queue is
+             * cleared.
+             */
+            time_diff = ble_att_svr_ticks_until_tmo(&conn->bhc_att_svr, now);
+            if (time_diff <= 0) {
+                /* Queued write has timed out.*/
+                ble_gap_terminate_with_conn(conn, BLE_ERR_REM_USER_CONN_TERM);
+                continue;
             }
-        }
 
-        ble_hs_unlock();
+            /* Determine if this connection is the soonest to time out. */
+            if (time_diff < next_exp_in) {
+                next_exp_in_new = time_diff;
+                next_exp_in_updated = true;
+            }
+#endif
 
-        /* If a connection has timed out, terminate it.  We need to repeatedly
-         * call this function again to determine when the next timeout is.
-         */
-        if (conn_handle != BLE_HS_CONN_HANDLE_NONE) {
-            ble_gap_terminate(conn_handle, BLE_ERR_REM_USER_CONN_TERM);
-            continue;
+            if (next_exp_in_updated) {
+                next_exp_in = next_exp_in_new;
+            }
         }
-
-        return next_exp_in;
     }
+
+    ble_hs_unlock();
+
+    return next_exp_in;
 }
 
 int
 ble_hs_conn_init(void)
 {

Vulnerable code:     ble_npl_time_t now;
    int32_t next_exp_in;
    uint16_t conn_handle;
    for (;;) {
        conn_handle = BLE_HS_CONN_HANDLE_NONE;
        next_exp_in = BLE_HS_FOREVER;
        now = ble_npl_time_get();
        ble_hs_lock();

        /* This loop performs one of two tasks:
         * 1. Determine if any connections need to be terminated due to timeout.
         *    If so, break out of the loop and terminate the connection.  This
         *    function will need to be executed again.
         * 2. Otherwise, determine when the next timeout will occur.
         */
        SLIST_FOREACH(conn, &ble_hs_conns, bhc_next) {
            if (!(conn->bhc_flags & BLE_HS_CONN_F_TERMINATING)) {
                /* Check each connection's rx fragment timer.  If too much time
                 * passes after a partial packet is received, the connection is
                 * terminated.
                 */
                if (conn->bhc_rx_chan != NULL) {
                    time_diff = conn->bhc_rx_timeout - now;

                    if (time_diff <= 0) {
                        /* ACL reassembly has timed out.  Remember the connection
                         * handle so it can be terminated after the mutex is
                         * unlocked.
                         */
                        conn_handle = conn->bhc_handle;
                        break;
                    }

                    /* Determine if this connection is the soonest to time out. */
                    if (time_diff < next_exp_in) {
                        next_exp_in = time_diff;
                    }
                }
#endif
#if BLE_HS_ATT_SVR_QUEUED_WRITE_TMO
                /* Check each connection's rx queued write timer.  If too much
                 * time passes after a prep write is received, the queue is
                 * cleared.
                 */
                time_diff = ble_att_svr_ticks_until_tmo(&conn->bhc_att_svr, now);
                    /* ACL reassembly has timed out.  Remember the connection
                     * handle so it can be terminated after the mutex is
                     * unlocked.
                     */
                    conn_handle = conn->bhc_handle;
                    break;
                    next_exp_in = time_diff;
        }
        ble_hs_unlock();
        /* If a connection has timed out, terminate it.  We need to repeatedly
         * call this function again to determine when the next timeout is.
         */
        if (conn_handle != BLE_HS_CONN_HANDLE_NONE) {
            ble_gap_terminate(conn_handle, BLE_ERR_REM_USER_CONN_TERM);
            continue;

        return next_exp_in;
Fixed code:     ble_npl_time_t now = ble_npl_time_get();
    int32_t next_exp_in = BLE_HS_FOREVER;
    int32_t next_exp_in_new;
    bool next_exp_in_updated;
    ble_hs_lock();
    /* This loop performs one of two tasks:
     * 1. Determine if any connections need to be terminated due to timeout. If
     *    so connection is disconnected.
     * 2. Otherwise, determine when the next timeout will occur.
     */
    SLIST_FOREACH(conn, &ble_hs_conns, bhc_next) {
        if (!(conn->bhc_flags & BLE_HS_CONN_F_TERMINATING)) {
            next_exp_in_updated = false;
            /* Check each connection's rx fragment timer.  If too much time
             * passes after a partial packet is received, the connection is
             * terminated.
             */
            if (conn->bhc_rx_chan != NULL) {
                time_diff = conn->bhc_rx_timeout - now;
                    /* ACL reassembly has timed out.*/
                    ble_gap_terminate_with_conn(conn, BLE_ERR_REM_USER_CONN_TERM);
                    continue;
                    next_exp_in_new = time_diff;
                    next_exp_in_updated = true;
            }

#if BLE_HS_ATT_SVR_QUEUED_WRITE_TMO
            /* Check each connection's rx queued write timer.  If too much
             * time passes after a prep write is received, the queue is
             * cleared.
             */
            time_diff = ble_att_svr_ticks_until_tmo(&conn->bhc_att_svr, now);
            if (time_diff <= 0) {
                /* Queued write has timed out.*/
                ble_gap_terminate_with_conn(conn, BLE_ERR_REM_USER_CONN_TERM);
                continue;
            /* Determine if this connection is the soonest to time out. */
            if (time_diff < next_exp_in) {
                next_exp_in_new = time_diff;
                next_exp_in_updated = true;
            }
#endif
            if (next_exp_in_updated) {
                next_exp_in = next_exp_in_new;
            }

    ble_hs_unlock();

    return next_exp_in;