CVE-2022-44640 HEIMDAL: asn1: invalid free in ASN.1 codec
authorNicolas Williams <nico@twosigma.com>
Wed, 10 Mar 2021 22:49:04 +0000 (16:49 -0600)
committerStefan Metzmacher <metze@samba.org>
Tue, 6 Dec 2022 13:41:05 +0000 (13:41 +0000)
Heimdal's ASN.1 compiler generates code that allows specially
crafted DER encodings of CHOICEs to invoke the wrong free function
on the decoded structure upon decode error.  This is known to impact
the Heimdal KDC, leading to an invalid free() of an address partly
or wholly under the control of the attacker, in turn leading to a
potential remote code execution (RCE) vulnerability.

This error affects the DER codec for all CHOICE types used in
Heimdal, though not all cases will be exploitable.  We have not
completed a thorough analysis of all the Heimdal components
affected, thus the Kerberos client, the X.509 library, and other
parts, may be affected as well.

This bug has been in Heimdal since 2005.  It was first reported by
Douglas Bagnall, though it had been found independently by the
Heimdal maintainers via fuzzing a few weeks earlier.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14929

(cherry-picked from Heimdal commit 9c9dac2b169255bad9071eea99fa90b980dde767)

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Tue Dec  6 13:41:05 UTC 2022 on sn-devel-184

third_party/heimdal/lib/asn1/fuzz-inputs/KrbFastArmoredReq [new file with mode: 0644]
third_party/heimdal/lib/asn1/gen_decode.c
third_party/heimdal/lib/asn1/gen_free.c
third_party/heimdal/lib/asn1/gen_template.c
third_party/heimdal/lib/asn1/krb5.asn1

diff --git a/third_party/heimdal/lib/asn1/fuzz-inputs/KrbFastArmoredReq b/third_party/heimdal/lib/asn1/fuzz-inputs/KrbFastArmoredReq
new file mode 100644 (file)
index 0000000..21ac360
Binary files /dev/null and b/third_party/heimdal/lib/asn1/fuzz-inputs/KrbFastArmoredReq differ
index 93d412f633561bde6e45646ac3392fcd3ce8577a..fa9d79a8ae5b32ff238bc39abce7047ed184f5c6 100644 (file)
@@ -694,14 +694,14 @@ decode_type(const char *name, const Type *t, int optional, struct value *defval,
                    classname(cl),
                    ty ? "CONS" : "PRIM",
                    valuename(cl, tag));
+           fprintf(codefile,
+                   "(%s)->element = %s;\n",
+                   name, m->label);
            if (asprintf (&s, "%s(%s)->u.%s", m->optional ? "" : "&",
                          name, m->gen_name) < 0 || s == NULL)
                errx(1, "malloc");
             decode_type(s, m->type, m->optional, NULL, forwstr, m->gen_name,
                         NULL, depth + 1);
-           fprintf(codefile,
-                   "(%s)->element = %s;\n",
-                   name, m->label);
            free(s);
            fprintf(codefile,
                    "}\n");
@@ -710,23 +710,23 @@ decode_type(const char *name, const Type *t, int optional, struct value *defval,
        if (have_ellipsis) {
            fprintf(codefile,
                    "else {\n"
+                   "(%s)->element = %s;\n"
                    "(%s)->u.%s.data = calloc(1, len);\n"
                    "if ((%s)->u.%s.data == NULL) {\n"
                    "e = ENOMEM; %s;\n"
                    "}\n"
                    "(%s)->u.%s.length = len;\n"
                    "memcpy((%s)->u.%s.data, p, len);\n"
-                   "(%s)->element = %s;\n"
                    "p += len;\n"
                    "ret += len;\n"
                    "len = 0;\n"
                    "}\n",
+                   name, have_ellipsis->label,
                    name, have_ellipsis->gen_name,
                    name, have_ellipsis->gen_name,
                    forwstr,
                    name, have_ellipsis->gen_name,
-                   name, have_ellipsis->gen_name,
-                   name, have_ellipsis->label);
+                   name, have_ellipsis->gen_name);
        } else {
            fprintf(codefile,
                    "else {\n"
index 53b7bfe7b9059972779e7056d5f9aa8d5c525291..14e6529046cf702b7299be30a08fa49b7a491396 100644 (file)
@@ -62,6 +62,13 @@ free_type (const char *name, const Type *t, int preserve)
     case TNull:
     case TGeneralizedTime:
     case TUTCTime:
+        /*
+         * This doesn't do much, but it leaves zeros where garbage might
+         * otherwise have been found.  Gets us closer to having the equivalent
+         * of a memset()-to-zero data structure after calling the free
+         * functions.
+         */
+        fprintf(codefile, "*%s = 0;\n", name);
        break;
     case TBitString:
        if (HEIM_TAILQ_EMPTY(t->members))
index e053a8bdd8bc4966865074dc5c47e781b8b08079..ad25fcfb29d3a29a0015a55c97822663e7410864 100644 (file)
@@ -1600,6 +1600,7 @@ generate_template(const Symbol *s)
            "int ASN1CALL\n"
            "decode_%s(const unsigned char *p, size_t len, %s *data, size_t *size)\n"
            "{\n"
+            "    memset(data, 0, sizeof(*data));\n"
            "    return _asn1_decode_top(asn1_%s, 0|%s, p, len, data, size);\n"
            "}\n"
            "\n",
index d7ce6bd6333d4cac1e7aebf33f767bdf8f95f061..00a0acbc029b63b7e7a1102b0b216ae7f3471109 100644 (file)
@@ -81,6 +81,7 @@ EXPORTS
        KrbFastFinished,
        KrbFastReq,
        KrbFastArmor,
+       KrbFastArmoredReq,
        KDCFastState,
        KDCFastCookie,
        KDC-PROXY-MESSAGE,