CVE-2017-15088: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: plugins/preauth/pkinit/pkinit_crypto_openssl.c in MIT Kerberos 5 (aka krb5) through 1.15.2 mishandles Distinguished Name (DN) fields, which allows remote attackers to execute arbitrary code or cause a denial of service (buffer overflow and application crash) in situations involving untrusted X.509 data, related to the get_matching_data and X509_NAME_oneline_ex functions. NOTE: this has security relevance only in use cases outside of the MIT Kerberos distribution, e.g., the use of get_matching_data in KDC certauth plugin code that is specific to Red Hat.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 9.8
Published: 2017-11-23T17:29Z
Repository: https://github.com/krb5/krb5
Language: C
Files changed: src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
Commit date: 2017-10-25 16:18:26 -0400
Commit message: Fix PKINIT cert matching data construction

Rewrite X509_NAME_oneline_ex() and its call sites to use dynamic
allocation and to perform proper error checking.

ticket: 8617
target_version: 1.16
target_version: 1.15-next
target_version: 1.14-next
tags: pullup

Diff: diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index f7640baf14c..ac107c2c1b6 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -5000,37 +5000,33 @@ check_kus:
     retval = 0;
 out:
     return retval;
 }
 
-/*
- * Return a string format of an X509_NAME in buf where
- * size is an in/out parameter.  On input it is the size
- * of the buffer, and on output it is the actual length
- * of the name.
- * If buf is NULL, returns the length req'd to hold name
- */
-static char *
-X509_NAME_oneline_ex(X509_NAME * a,
-                     char *buf,
-                     unsigned int *size,
-                     unsigned long flag)
+static krb5_error_code
+rfc2253_name(X509_NAME *name, char **str_out)
 {
-    BIO *out = NULL;
+    BIO *b = NULL;
+    char *str;
 
-    out = BIO_new(BIO_s_mem ());
-    if (X509_NAME_print_ex(out, a, 0, flag) > 0) {
-        if (buf != NULL && (*size) >  (unsigned int) BIO_number_written(out)) {
-            memset(buf, 0, *size);
-            BIO_read(out, buf, (int) BIO_number_written(out));
-        }
-        else {
-            *size = BIO_number_written(out);
-        }
-    }
-    BIO_free(out);
-    return (buf);
+    *str_out = NULL;
+    b = BIO_new(BIO_s_mem());
+    if (b == NULL)
+        return ENOMEM;
+    if (X509_NAME_print_ex(b, name, 0, XN_FLAG_SEP_COMMA_PLUS) < 0)
+        goto error;
+    str = calloc(BIO_number_written(b) + 1, 1);
+    if (str == NULL)
+        goto error;
+    BIO_read(b, str, BIO_number_written(b));
+    BIO_free(b);
+    *str_out = str;
+    return 0;
+
+error:
+    BIO_free(b);
+    return ENOMEM;
 }
 
 /*
  * Get number of certificates available after crypto_load_certs()
  */
@@ -5092,36 +5088,23 @@ get_matching_data(krb5_context context,
 {
     krb5_error_code ret = ENOMEM;
     pkinit_cert_matching_data *md = NULL;
     krb5_principal *pkinit_sans = NULL, *upn_sans = NULL;
     size_t i, j;
-    char buf[DN_BUF_LEN];
-    unsigned int bufsize = sizeof(buf);
 
     *md_out = NULL;
 
     md = calloc(1, sizeof(*md));
     if (md == NULL)
         goto cleanup;
 
-    /* Get the subject name (in rfc2253 format). */
-    X509_NAME_oneline_ex(X509_get_subject_name(cert), buf, &bufsize,
-                         XN_FLAG_SEP_COMMA_PLUS);
-    md->subject_dn = strdup(buf);
-    if (md->subject_dn == NULL) {
-        ret = ENOMEM;
+    ret = rfc2253_name(X509_get_subject_name(cert), &md->subject_dn);
+    if (ret)
         goto cleanup;
-    }
-
-    /* Get the issuer name (in rfc2253 format). */
-    X509_NAME_oneline_ex(X509_get_issuer_name(cert), buf, &bufsize,
-                         XN_FLAG_SEP_COMMA_PLUS);
-    md->issuer_dn = strdup(buf);
-    if (md->issuer_dn == NULL) {
-        ret = ENOMEM;
+    ret = rfc2253_name(X509_get_issuer_name(cert), &md->issuer_dn);
+    if (ret)
         goto cleanup;
-    }
 
     /* Get the SAN data. */
     ret = crypto_retrieve_X509_sans(context, plg_cryptoctx, req_cryptoctx,
                                     cert, &pkinit_sans, &upn_sans, NULL);
     if (ret)

Vulnerable code: /*
 * Return a string format of an X509_NAME in buf where
 * size is an in/out parameter.  On input it is the size
 * of the buffer, and on output it is the actual length
 * of the name.
 * If buf is NULL, returns the length req'd to hold name
 */
static char *
X509_NAME_oneline_ex(X509_NAME * a,
                     char *buf,
                     unsigned int *size,
                     unsigned long flag)
    BIO *out = NULL;
    out = BIO_new(BIO_s_mem ());
    if (X509_NAME_print_ex(out, a, 0, flag) > 0) {
        if (buf != NULL && (*size) >  (unsigned int) BIO_number_written(out)) {
            memset(buf, 0, *size);
            BIO_read(out, buf, (int) BIO_number_written(out));
        }
        else {
            *size = BIO_number_written(out);
        }
    }
    BIO_free(out);
    return (buf);
    char buf[DN_BUF_LEN];
    unsigned int bufsize = sizeof(buf);
    /* Get the subject name (in rfc2253 format). */
    X509_NAME_oneline_ex(X509_get_subject_name(cert), buf, &bufsize,
                         XN_FLAG_SEP_COMMA_PLUS);
    md->subject_dn = strdup(buf);
    if (md->subject_dn == NULL) {
        ret = ENOMEM;
    }

    /* Get the issuer name (in rfc2253 format). */
    X509_NAME_oneline_ex(X509_get_issuer_name(cert), buf, &bufsize,
                         XN_FLAG_SEP_COMMA_PLUS);
    md->issuer_dn = strdup(buf);
    if (md->issuer_dn == NULL) {
        ret = ENOMEM;
    }
Fixed code: static krb5_error_code
rfc2253_name(X509_NAME *name, char **str_out)
    BIO *b = NULL;
    char *str;
    *str_out = NULL;
    b = BIO_new(BIO_s_mem());
    if (b == NULL)
        return ENOMEM;
    if (X509_NAME_print_ex(b, name, 0, XN_FLAG_SEP_COMMA_PLUS) < 0)
        goto error;
    str = calloc(BIO_number_written(b) + 1, 1);
    if (str == NULL)
        goto error;
    BIO_read(b, str, BIO_number_written(b));
    BIO_free(b);
    *str_out = str;
    return 0;

error:
    BIO_free(b);
    return ENOMEM;
    ret = rfc2253_name(X509_get_subject_name(cert), &md->subject_dn);
    if (ret)
    ret = rfc2253_name(X509_get_issuer_name(cert), &md->issuer_dn);
    if (ret)