[Security] Use-after-free in gnutls_pkcs11_token_set_pin when retrieving SO PIN

Hi GnuTLS maintainers,

We believe that we have discovered a potential security vulnerability in gnutls when handling PKCS#11 Security Officer (SO) PIN changes without an existing PIN on tokens lacking a protected authentication path.

Vulnerability Details When gnutls_pkcs11_token_set_pin() is invoked with GNUTLS_PIN_SO, oldpin == NULL, and the token does not advertise CKF_PROTECTED_AUTHENTICATION_PATH, the parsed PKCS#11 URI is freed immediately after opening the session, but later reused inside the SO PIN retrieval path:

ret = pkcs11_open_session(&sinfo, NULL, info, ses_flags);
p11_kit_uri_free(info);                 /* info freed */
...
ret = pkcs11_retrieve_pin(&pin_info, info, &sinfo.tinfo, 0, CKU_SO, &pin);

pkcs11_retrieve_pin() dereferences the freed URI to consult pin-value / pin-source, leading to a heap use-after-free:

pinfile = p11_kit_uri_get_pin_value(info);
if (pinfile == NULL)
    pinfile = p11_kit_uri_get_pin_source(info);

This is a classical CWE-416 (Use After Free) that can manifest as process crashes or, under precise heap manipulation, memory corruption. Proposed Fix A minimal repair is to defer freeing the URI until after all uses, routing early error paths through a common cleanup:

diff --git a/lib/pkcs11_write.c b/lib/pkcs11_write.c
@@
-    ret = pkcs11_open_session(&sinfo, NULL, info, ses_flags);
-    p11_kit_uri_free(info);
-
-    if (ret < 0) {
-        gnutls_assert();
-        return ret;
-    }
+    ret = pkcs11_open_session(&sinfo, NULL, info, ses_flags);
+    if (ret < 0) {
+        gnutls_assert();
+        goto finish;
+    }
+    session_open = 1;
@@
-            if (newpin == NULL)
-                return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
+            if (newpin == NULL) {
+                ret = gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
+                goto finish;
+            }
@@
-finish:
-    pkcs11_close_session(&sinfo);
+finish:
+    p11_kit_uri_free(info);
+    if (session_open)
+        pkcs11_close_session(&sinfo);
     return ret;

(Any equivalent patch that preserves info until after pkcs11_retrieve_pin() executes would address the issue.)

Best wishes, Luigino Camastra Aisle Research

Assignee Loading
Time tracking Loading