Length check when parsing GSS token encapsulation

gssint_get_mech_type_oid() is used by gss_accept_sec_context() to
determine the mechanism of the token.  Without length checking, it
might read a few bytes past the end of the input token buffer.  Add
length checking as well as test cases for truncated encapsulations.
Reported by Bar Katz.

ticket: 8620 (new)
target_version: 1.16
target_version: 1.15-next
target_version: 1.14-next
tags: pullup

diff --git a/src/lib/gssapi/mechglue/g_glue.c b/src/lib/gssapi/mechglue/g_glue.c
index 4aa3591a0d066edd214c96204d53c3417d0e291d..4cd2e8f8eb8c2a2fddab9290fe12e985f3767380 100644 (file)
--- a/src/lib/gssapi/mechglue/g_glue.c
+++ b/src/lib/gssapi/mechglue/g_glue.c
@@ -189,7 +189,7 @@ OM_uint32 gssint_get_mech_type_oid(OID, token)
     gss_buffer_t       token;
 {
     unsigned char * buffer_ptr;
-    int length;
+    size_t buflen, lenbytes, length, oidlen;
 
     /*
      * This routine reads the prefix of "token" in order to determine
@@ -223,25 +223,33 @@ OM_uint32 gssint_get_mech_type_oid(OID, token)
     /* Skip past the APP/Sequnce byte and the token length */
 
     buffer_ptr = (unsigned char *) token->value;
+    buflen = token->length;
 
-    if (*(buffer_ptr++) != 0x60)
+    if (buflen < 2 || *buffer_ptr++ != 0x60)
        return (GSS_S_DEFECTIVE_TOKEN);
     length = *buffer_ptr++;
+    buflen -= 2;
 
        /* check if token length is null */
        if (length == 0)
            return (GSS_S_DEFECTIVE_TOKEN);
 
     if (length & 0x80) {
-       if ((length & 0x7f) > 4)
+       lenbytes = length & 0x7f;
+       if (lenbytes > 4 || lenbytes > buflen)
            return (GSS_S_DEFECTIVE_TOKEN);
-       buffer_ptr += length & 0x7f;
+       buffer_ptr += lenbytes;
+       buflen -= lenbytes;
     }
 
-    if (*(buffer_ptr++) != 0x06)
+    if (buflen < 2 || *buffer_ptr++ != 0x06)
+       return (GSS_S_DEFECTIVE_TOKEN);
+    oidlen = *buffer_ptr++;
+    buflen -= 2;
+    if (oidlen > 0x7f || oidlen > buflen)
        return (GSS_S_DEFECTIVE_TOKEN);
 
-    OID->length = (OM_uint32) *(buffer_ptr++);
+    OID->length = oidlen;
     OID->elements = (void *) buffer_ptr;
     return (GSS_S_COMPLETE);
 }

diff --git a/src/tests/gssapi/t_invalid.c b/src/tests/gssapi/t_invalid.c
index 5c8ddac8dc8467b7fe1e64ef44bd9e77cfd4cd43..2a332a8ae04852a8b0d7ddb5e69a37c455ef797a 100644 (file)
--- a/src/tests/gssapi/t_invalid.c
+++ b/src/tests/gssapi/t_invalid.c
@@ -31,8 +31,8 @@
  */
 
 /*
- * This file contains regression tests for some GSSAPI krb5 invalid per-message
- * token vulnerabilities.
+ * This file contains regression tests for some GSSAPI invalid token
+ * vulnerabilities.
  *
  * 1. A pre-CFX wrap or MIC token processed with a CFX-only context causes a
  *    null pointer dereference.  (The token must use SEAL_ALG_NONE or it will
@@ -54,10 +54,13 @@
  *    causes an integer underflow when computing the original message length,
  *    leading to an allocation error.
  *
+ * 5. In the mechglue, truncated encapsulation in the initial context token can
+ *    cause input buffer overruns in gss_accept_sec_context().
+ *
  * Vulnerabilities #1 and #2 also apply to IOV unwrap, although tokens with
- * fewer than 16 bytes after the ASN.1 header will be rejected.  Vulnerability
- * #2 can only be robustly detected using a memory-checking environment such as
- * valgrind.
+ * fewer than 16 bytes after the ASN.1 header will be rejected.
+ * Vulnerabilities #2 and #5 can only be robustly detected using a
+ * memory-checking environment such as valgrind.
  */
 
 #include "k5-int.h"
@@ -406,6 +409,48 @@ test_bad_pad(gss_ctx_id_t ctx, const struct test *test)
     (void)gss_release_buffer(&minor, &out);
 }
 
+static void
+try_accept(void *value, size_t len)
+{
+    OM_uint32 minor;
+    gss_buffer_desc in, out;
+    gss_ctx_id_t ctx = GSS_C_NO_CONTEXT;
+
+    /* Copy the provided value to make input overruns more obvious. */
+    in.value = malloc(len);
+    if (in.value == NULL)
+        abort();
+    memcpy(in.value, value, len);
+    in.length = len;
+    (void)gss_accept_sec_context(&minor, &ctx, GSS_C_NO_CREDENTIAL, &in,
+                                 GSS_C_NO_CHANNEL_BINDINGS, NULL, NULL,
+                                 &out, NULL, NULL, NULL);
+    gss_release_buffer(&minor, &out);
+    gss_delete_sec_context(&minor, &ctx, GSS_C_NO_BUFFER);
+    free(in.value);
+}
+
+/* Accept contexts using superficially valid but truncated encapsulations. */
+static void
+test_short_encapsulation()
+{
+    /* Include just the initial application tag, to see if we overrun reading
+     * the sequence length. */
+    try_accept("\x60", 1);
+
+    /* Indicate four additional sequence length bytes, to see if we overrun
+     * reading them (or skipping them and reading the next byte). */
+    try_accept("\x60\x84", 2);
+
+    /* Include an object identifier tag but no length, to see if we overrun
+     * reading the length. */
+    try_accept("\x60\x40\x06", 3);
+
+    /* Include an object identifier tag with a length matching the krb5 mech,
+     * but no OID bytes, to see if we overrun comparing against mechs. */
+    try_accept("\x60\x40\x06\x09", 4);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -425,5 +470,7 @@ main(int argc, char **argv)
         free_fake_context(ctx);
     }
 
+    test_short_encapsulation();
+
     return 0;
 }