From f8b42ef541a463f56720ec9358dd07716b04c5e2 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Thu, 22 May 2014 22:31:26 -0400
Subject: Properly handle PKCS11 label in PKINIT

The CK_TOKEN_INFO label field is defined to be zero-filled, but it may
not be zero-terminated if all bytes of the field are used.  Use only
length-counted operations to process it.  Also avoid underrunning the
buffer pointer if the label is empty or contains only whitespace.

ticket: 7917
target_version: 1.12.2
tags: pullup
---
 src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 28 ++++++++++++++--------
 1 file changed, 18 insertions(+), 10 deletions(-)

(limited to 'src/plugins')

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index 109de23f9..1d6b0cd7a 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -3738,6 +3738,7 @@ pkinit_open_session(krb5_context context,
 {
     CK_ULONG i, r;
     unsigned char *cp;
+    size_t label_len;
     CK_ULONG count = 0;
     CK_SLOT_ID_PTR slotlist;
     CK_TOKEN_INFO tinfo;
@@ -3788,13 +3789,20 @@ pkinit_open_session(krb5_context context,
             pkiDebug("C_GetTokenInfo: %s\n", pkinit_pkcs11_code_to_text(r));
             return KRB5KDC_ERR_PREAUTH_FAILED;
         }
-        for (cp = tinfo.label + sizeof (tinfo.label) - 1;
-             *cp == '\0' || *cp == ' '; cp--)
-            *cp = '\0';
-        pkiDebug("open_session: slotid %d token \"%s\"\n",
-                 (int) slotlist[i], tinfo.label);
+
+        /* tinfo.label is zero-filled but not necessarily zero-terminated.
+         * Find the length, ignoring any trailing spaces. */
+        for (cp = tinfo.label + sizeof(tinfo.label); cp > tinfo.label; cp--) {
+            if (cp[-1] != '\0' && cp[-1] != ' ')
+                break;
+        }
+        label_len = cp - tinfo.label;
+
+        pkiDebug("open_session: slotid %d token \"%.*s\"\n",
+                 (int)slotlist[i], (int)label_len, tinfo.label);
         if (cctx->token_label == NULL ||
-            !strcmp((char *) cctx->token_label, (char *) tinfo.label))
+            (strlen(cctx->token_label) == label_len &&
+             memcmp(cctx->token_label, tinfo.label, label_len) == 0))
             break;
         cctx->p11->C_CloseSession(cctx->session);
     }
@@ -3813,15 +3821,15 @@ pkinit_open_session(krb5_context context,
         if (cctx->p11_module_name != NULL) {
             if (cctx->slotid != PK_NOSLOT) {
                 if (asprintf(&p11name,
-                             "PKCS11:module_name=%s:slotid=%ld:token=%s",
+                             "PKCS11:module_name=%s:slotid=%ld:token=%.*s",
                              cctx->p11_module_name, (long)cctx->slotid,
-                             tinfo.label) < 0)
+                             (int)label_len, tinfo.label) < 0)
                     p11name = NULL;
             } else {
                 if (asprintf(&p11name,
-                             "PKCS11:module_name=%s,token=%s",
+                             "PKCS11:module_name=%s,token=%.*s",
                              cctx->p11_module_name,
-                             tinfo.label) < 0)
+                             (int)label_len, tinfo.label) < 0)
                     p11name = NULL;
             }
         } else {
-- 
cgit