CVE-2020-28196: Uncontrolled Recursion (CWE-674)
value: MIT Kerberos 5 (aka krb5) before 1.17.2 and 1.18.x before 1.18.3 allows unbounded recursion via an ASN.1-encoded Kerberos message because the lib/krb5/asn.1/asn1_encode.c support for BER indefinite lengths lacks a recursion limit.
CWE description: The product does not properly control the amount of recursion that takes place,  consuming excessive resources, such as allocated memory or the program stack.
CVSS v3 base score: 7.5
Published: 2020-11-06T08:15Z
Repository: https://github.com/krb5/krb5
Language: C
Files changed: src/lib/krb5/asn.1/asn1_encode.c
Commit date: 2020-11-03 15:20:25 -0500
Commit message: Add recursion limit for ASN.1 indefinite lengths

The libkrb5 ASN.1 decoder supports BER indefinite lengths.  It
computes the tag length using recursion; the lack of a recursion limit
allows an attacker to overrun the stack and cause the process to
crash.  Reported by Demi Obenour.

CVE-2020-28196:

In MIT krb5 releases 1.11 and later, an unauthenticated attacker can
cause a denial of service for any client or server to which it can
send an ASN.1-encoded Kerberos message of sufficient length.

ticket: 8959 (new)
tags: pullup
target_version: 1.18-next
target_version: 1.17-next

Diff: diff --git a/src/lib/krb5/asn.1/asn1_encode.c b/src/lib/krb5/asn.1/asn1_encode.c
index a160cf4fe86..cd6b879f776 100644
--- a/src/lib/krb5/asn.1/asn1_encode.c
+++ b/src/lib/krb5/asn.1/asn1_encode.c
@@ -354,11 +354,11 @@ make_tag(asn1buf *buf, const taginfo *t, size_t len)
  * However, we still insist on the primitive form of string types.)
  */
 static krb5_error_code
 get_tag(const uint8_t *asn1, size_t len, taginfo *tag_out,
         const uint8_t **contents_out, size_t *clen_out,
-        const uint8_t **remainder_out, size_t *rlen_out)
+        const uint8_t **remainder_out, size_t *rlen_out, int recursion)
 {
     krb5_error_code ret;
     uint8_t o;
     const uint8_t *c, *p, *tag_start = asn1;
     size_t clen, llen, i;
@@ -392,13 +392,15 @@ get_tag(const uint8_t *asn1, size_t len, taginfo *tag_out,
 
     if (o == 0x80) {
         /* Indefinite form (should not be present in DER, but we accept it). */
         if (tag_out->construction != CONSTRUCTED)
             return ASN1_MISMATCH_INDEF;
+        if (recursion >= 32)
+            return ASN1_OVERFLOW;
         p = asn1;
         while (!(len >= 2 && p[0] == 0 && p[1] == 0)) {
-            ret = get_tag(p, len, &t, &c, &clen, &p, &len);
+            ret = get_tag(p, len, &t, &c, &clen, &p, &len, recursion + 1);
             if (ret)
                 return ret;
         }
         tag_out->tag_end_len = 2;
         *contents_out = asn1;
@@ -611,11 +613,11 @@ split_der(asn1buf *buf, uint8_t *const *der, size_t len, taginfo *tag_out)
 {
     krb5_error_code ret;
     const uint8_t *contents, *remainder;
     size_t clen, rlen;
 
-    ret = get_tag(*der, len, tag_out, &contents, &clen, &remainder, &rlen);
+    ret = get_tag(*der, len, tag_out, &contents, &clen, &remainder, &rlen, 0);
     if (ret)
         return ret;
     if (rlen != 0)
         return ASN1_BAD_LENGTH;
     insert_bytes(buf, contents, clen);
@@ -1197,11 +1199,11 @@ decode_atype(const taginfo *t, const uint8_t *asn1, size_t len,
         taginfo inner_tag;
         const taginfo *tp = t;
         const uint8_t *rem;
         size_t rlen;
         if (!tag->implicit) {
-            ret = get_tag(asn1, len, &inner_tag, &asn1, &len, &rem, &rlen);
+            ret = get_tag(asn1, len, &inner_tag, &asn1, &len, &rem, &rlen, 0);
             if (ret)
                 return ret;
             /* Note: we don't check rlen (it should be 0). */
             tp = &inner_tag;
             if (!check_atype_tag(tag->basetype, tp))
@@ -1418,11 +1420,11 @@ decode_sequence(const uint8_t *asn1, size_t len, const struct seq_info *seq,
 
     assert(seq->n_fields > 0);
     for (i = 0; i < seq->n_fields; i++) {
         if (len == 0)
             break;
-        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len);
+        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len, 0);
         if (ret)
             goto error;
         /*
          * Find the applicable sequence field.  This logic is a little
          * oversimplified; we could match an element to an optional extensible
@@ -1476,11 +1478,11 @@ decode_sequence_of(const uint8_t *asn1, size_t len,
     taginfo t;
 
     *seq_out = NULL;
     *count_out = 0;
     while (len > 0) {
-        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len);
+        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len, 0);
         if (ret)
             goto error;
         if (!check_atype_tag(elemtype, &t)) {
             ret = ASN1_BAD_ID;
             goto error;
@@ -1582,11 +1584,11 @@ k5_asn1_full_decode(const krb5_data *code, const struct atype_info *a,
     size_t clen, rlen;
     taginfo t;
 
     *retrep = NULL;
     ret = get_tag((uint8_t *)code->data, code->length, &t, &contents,
-                  &clen, &remainder, &rlen);
+                  &clen, &remainder, &rlen, 0);
     if (ret)
         return ret;
     /* rlen should be 0, but we don't check it (and due to padding in
      * non-length-preserving enctypes, it will sometimes be nonzero). */
     if (!check_atype_tag(a, &t))

Vulnerable code:         const uint8_t **remainder_out, size_t *rlen_out)
            ret = get_tag(p, len, &t, &c, &clen, &p, &len);
    ret = get_tag(*der, len, tag_out, &contents, &clen, &remainder, &rlen);
            ret = get_tag(asn1, len, &inner_tag, &asn1, &len, &rem, &rlen);
        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len);
        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len);
                  &clen, &remainder, &rlen);
Fixed code:         const uint8_t **remainder_out, size_t *rlen_out, int recursion)
        if (recursion >= 32)
            return ASN1_OVERFLOW;
            ret = get_tag(p, len, &t, &c, &clen, &p, &len, recursion + 1);
    ret = get_tag(*der, len, tag_out, &contents, &clen, &remainder, &rlen, 0);
            ret = get_tag(asn1, len, &inner_tag, &asn1, &len, &rem, &rlen, 0);
        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len, 0);
        ret = get_tag(asn1, len, &t, &contents, &clen, &asn1, &len, 0);
                  &clen, &remainder, &rlen, 0);