krb5 commit: Add various bound checks

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/277f9531745c45f14cce729b477e46219334d613
commit 277f9531745c45f14cce729b477e46219334d613
Author: Martin Kittel <[email protected]>
Date:   Thu Apr 6 21:03:23 2017 +0200

    Add various bound checks
    
    Add bounds checks where Coverity otherwise reports a defect.  Most of
    these checks are unlikely to be triggered in practice (Unicode regexps
    are unused, and the caller of gss_krb5int_make_seal_token_v3 won't
    have a plaintext object larger than half of the address space).  The
    checks in dump.c could prevent memory access errors resulting from a
    malformed dump file.
    
    [[email protected]: rewrote commit message]
    
    ticket: 8578 (new)

 src/kadmin/dbutil/dump.c       |   14 +++++++++++++-
 src/lib/gssapi/krb5/k5sealv3.c |    8 +++++++-
 src/lib/kdb/kdb_default.c      |    2 +-
 src/lib/krb5/unicode/ure/ure.c |    2 +-
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index f7889bd..9f5c3d1 100644
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -688,6 +688,10 @@ process_tl_data(const char *fname, FILE *filep, int lineno,
                      _("cannot read tagged data type and length"));
             return EINVAL;
         }
+        if (i1 < INT16_MIN || i1 > INT16_MAX || u1 > UINT16_MAX) {
+            load_err(fname, lineno, _("data type or length overflowed"));
+            return EINVAL;
+        }
         tl->tl_data_type = i1;
         tl->tl_data_length = u1;
         if (read_octets_or_minus1(filep, tl->tl_data_length,
@@ -735,6 +739,10 @@ process_k5beta7_princ(krb5_context context, const char *fname, FILE *filep,
         goto fail;
 
     /* Get memory for and form tagged data linked list */
+    if (u3 > UINT16_MAX) {
+        load_err(fname, *linenop, _("cannot allocate tl_data (too large)"));
+        goto fail;
+    }
     if (alloc_tl_data(u3, &dbentry->tl_data))
         goto fail;
     dbentry->n_tl_data = u3;
@@ -823,13 +831,17 @@ process_k5beta7_princ(krb5_context context, const char *fname, FILE *filep,
             load_err(fname, *linenop, _("cannot read key size and version"));
             goto fail;
         }
+        if (t1 > KRB5_KDB_V1_KEY_DATA_ARRAY) {
+            load_err(fname, *linenop, _("unsupported key_data_ver version"));
+            goto fail;
+        }
 
         kd->key_data_ver = t1;
         kd->key_data_kvno = t2;
 
         for (j = 0; j < t1; j++) {
             nread = fscanf(filep, "%d\t%d\t", &t3, &t4);
-            if (nread != 2) {
+            if (nread != 2 || t4 < 0) {
                 load_err(fname, *linenop,
                          _("cannot read key type and length"));
                 goto fail;
diff --git a/src/lib/gssapi/krb5/k5sealv3.c b/src/lib/gssapi/krb5/k5sealv3.c
index 0038a8e..25d9f27 100644
--- a/src/lib/gssapi/krb5/k5sealv3.c
+++ b/src/lib/gssapi/krb5/k5sealv3.c
@@ -110,6 +110,7 @@ gss_krb5int_make_seal_token_v3 (krb5_context context,
         krb5_data plain;
         krb5_enc_data cipher;
         size_t ec_max;
+        size_t encrypt_size;
 
         /* 300: Adds some slop.  */
         if (SIZE_MAX - 300 < message->length)
@@ -128,7 +129,12 @@ gss_krb5int_make_seal_token_v3 (krb5_context context,
             return err;
 
         /* Get size of ciphertext.  */
-        bufsize = 16 + krb5_encrypt_size (plain.length, key->keyblock.enctype);
+        encrypt_size = krb5_encrypt_size(plain.length, key->keyblock.enctype);
+        if (encrypt_size > SIZE_MAX / 2) {
+            err = ENOMEM;
+            goto error;
+        }
+        bufsize = 16 + encrypt_size;
         /* Allocate space for header plus encrypted data.  */
         outbuf = gssalloc_malloc(bufsize);
         if (outbuf == NULL) {
diff --git a/src/lib/kdb/kdb_default.c b/src/lib/kdb/kdb_default.c
index 7a75148..a1021f1 100644
--- a/src/lib/kdb/kdb_default.c
+++ b/src/lib/kdb/kdb_default.c
@@ -282,7 +282,7 @@ krb5_db_def_fetch_mkey_stash(krb5_context   context,
     key->length = keylength;
 #endif
 
-    if (!key->length || ((int) key->length) < 0) {
+    if (!key->length || key->length > 1024) {
         retval = KRB5_KDB_BADSTORED_MKEY;
         goto errout;
     }
diff --git a/src/lib/krb5/unicode/ure/ure.c b/src/lib/krb5/unicode/ure/ure.c
index d1cfd8a..23a03d9 100644
--- a/src/lib/krb5/unicode/ure/ure.c
+++ b/src/lib/krb5/unicode/ure/ure.c
@@ -421,7 +421,7 @@ _ure_prop_list(ucs2_t *pp, unsigned long limit, unsigned long *mask,
           b->error = _URE_INVALID_PROPERTY;
     }
 
-    if (n != 0)
+    if (b->error == _URE_OK && n != 0)
       m |= cclass_flags[n];
 
     /*
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.