CVE-2022-44640 HEIMDAL: asn1: invalid free in ASN.1 codec

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/gen_decode.c

@@ -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"

third_party/heimdal/lib/asn1/gen_free.c

@@ -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))

third_party/heimdal/lib/asn1/gen_template.c

@@ -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",

third_party/heimdal/lib/asn1/krb5.asn1

@@ -81,6 +81,7 @@ EXPORTS
 	KrbFastFinished,
 	KrbFastReq,
 	KrbFastArmor,
+	KrbFastArmoredReq,
 	KDCFastState,
 	KDCFastCookie,
 	KDC-PROXY-MESSAGE,