From 1fd7907279521720449eed2f116b6f32c10bd220 Mon Sep 17 00:00:00 2001
From: Andrew Hutchings <andrew@wolfssl.com>
Date: Thu, 15 Oct 2026 16:22:20 +0100
Subject: [PATCH] Fix use-after-free in WP11_Object_Find

WP11_Object_Find walked the object list under the token lock, released
the lock, and then read obj->handle and obj->opFlag. A concurrent
C_DestroyObject of the same token object from another session could
unlink and free the object in that window, which ASAN caught in
concurrent_destroy_object_test.

Read everything needed from the object, including the CKA_PRIVATE flag,
while the token lock is held, and do not dereference it after the
unlock. The slot lock used for the login state check is still only
taken after the token lock is released, so the slot-then-token lock
order is unchanged.
---
 src/internal.c | 49 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/src/internal.c b/src/internal.c
index 6fa7d2a4..f3306b5e 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -12065,22 +12065,27 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle,
     WP11_Object* obj;
 #ifdef WOLFPKCS11_NSS
     WP11_Session* scan;
+#else
+    int isPrivate = 0;
 #endif
     int onToken = OBJ_HANDLE_ON_TOKEN(objHandle);
 
-    if (!onToken) {
 #ifdef WOLFPKCS11_NSS
-        /* The NSS cross-session walk below needs the slot lock so a concurrent
-         * remove-session can't reclaim a session node. Acquire it before the
-         * token lock to match the global lock order (slot then token) used by
-         * WP11_Session_RemoveObjectByHandle / WP11_Slot_CloseSessions; acquiring
-         * them in the opposite order would risk an AB-BA deadlock. */
+    /* The NSS cross-session walk below needs the slot lock so a concurrent
+     * remove-session can't reclaim a session node. Acquire it before the
+     * token lock to match the global lock order (slot then token) used by
+     * WP11_Session_RemoveObjectByHandle / WP11_Slot_CloseSessions; acquiring
+     * them in the opposite order would risk an AB-BA deadlock. */
+    if (!onToken)
         WP11_Lock_LockRO(&session->slot->lock);
 #endif
-        /* Hold the token lock across the session->object walk so a concurrent
-         * WP11_Session_RemoveObjectByHandle (which holds the same lock around
-         * the unlink) can't free a node we're stepping through. */
-        WP11_Lock_LockRO(&session->slot->token.lock);
+    /* The token lock guards every object list on the slot, and
+     * WP11_Session_RemoveObjectByHandle holds it around the unlink. Hold it
+     * across the walk and every read of the matched object: once it is
+     * released a concurrent C_DestroyObject on the same handle (token objects
+     * are shared by all sessions) can unlink and free the object. */
+    WP11_Lock_LockRO(&session->slot->token.lock);
+    if (!onToken) {
         obj = session->object;
         while (obj != NULL) {
             if (obj->handle == objHandle) {
@@ -12103,14 +12108,9 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle,
                 }
             }
         }
-#endif
-        WP11_Lock_UnlockRO(&session->slot->token.lock);
-#ifdef WOLFPKCS11_NSS
-        WP11_Lock_UnlockRO(&session->slot->lock);
 #endif
     }
     else {
-        WP11_Lock_LockRO(&session->slot->token.lock);
         obj = session->slot->token.object;
         while (obj != NULL) {
             if (obj->handle == objHandle) {
@@ -12119,15 +12119,26 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle,
             }
             obj = obj->next;
         }
-        WP11_Lock_UnlockRO(&session->slot->token.lock);
     }
+#ifndef WOLFPKCS11_NSS
+    if (ret == 0)
+        isPrivate = ((obj->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE);
+#endif
+    WP11_Lock_UnlockRO(&session->slot->token.lock);
+#ifdef WOLFPKCS11_NSS
+    if (!onToken)
+        WP11_Lock_UnlockRO(&session->slot->lock);
+#endif
+    /* obj may already be freed by a concurrent destroy: do not dereference it
+     * from here on. */
 
-    if (ret == 0 && obj != NULL && (obj->handle == objHandle)) {
+    if (ret == 0) {
 #ifndef WOLFPKCS11_NSS
         /* Enforce CKA_PRIVATE: reject private objects from public sessions.
          * Skipped in NSS mode because NSS operates as the internal crypto
-         * module without calling C_Login. */
-        if ((obj->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) {
+         * module without calling C_Login. The slot lock is only taken after
+         * the token lock is released, keeping the slot-then-token order. */
+        if (isPrivate) {
             int loginState;
             WP11_Lock_LockRO(&session->slot->lock);
             loginState = session->slot->token.loginState;