Fix error handling in x509v3_cache_extensions and related functions

Basically we use EXFLAG_INVALID for all kinds of out of memory and
all kinds of parse errors in x509v3_cache_extensions.

[extended tests]

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10755)

1 files changed, 68 insertions, 29 deletions

diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 10fd0f73f3..3c39c8fda7 100644
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1999-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1999-2020 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -81,6 +81,8 @@ int X509_check_purpose(X509 *x, int id, int ca)
     const X509_PURPOSE *pt;
 
     x509v3_cache_extensions(x);
+    if (x->ex_flags & EXFLAG_INVALID)
+        return -1;
 
     /* Return if side-effect only call */
     if (id == -1)
@@ -300,10 +302,11 @@ int X509_supported_extension(X509_EXTENSION *ex)
     return 0;
 }
 
-static void setup_dp(X509 *x, DIST_POINT *dp)
+static int setup_dp(X509 *x, DIST_POINT *dp)
 {
     X509_NAME *iname = NULL;
     int i;
+
     if (dp->reasons) {
         if (dp->reasons->length > 0)
             dp->dp_reasons = dp->reasons->data[0];
@@ -313,7 +316,7 @@ static void setup_dp(X509 *x, DIST_POINT *dp)
     } else
         dp->dp_reasons = CRLDP_ALL_REASONS;
     if (!dp->distpoint || (dp->distpoint->type != 1))
-        return;
+        return 1;
     for (i = 0; i < sk_GENERAL_NAME_num(dp->CRLissuer); i++) {
         GENERAL_NAME *gen = sk_GENERAL_NAME_value(dp->CRLissuer, i);
         if (gen->type == GEN_DIRNAME) {
@@ -324,16 +327,21 @@ static void setup_dp(X509 *x, DIST_POINT *dp)
     if (!iname)
         iname = X509_get_issuer_name(x);
 
-    DIST_POINT_set_dpname(dp->distpoint, iname);
-
+    return DIST_POINT_set_dpname(dp->distpoint, iname);
 }
 
-static void setup_crldp(X509 *x)
+static int setup_crldp(X509 *x)
 {
     int i;
-    x->crldp = X509_get_ext_d2i(x, NID_crl_distribution_points, NULL, NULL);
-    for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++)
-        setup_dp(x, sk_DIST_POINT_value(x->crldp, i));
+
+    x->crldp = X509_get_ext_d2i(x, NID_crl_distribution_points, &i, NULL);
+    if (x->crldp == NULL && i != -1)
+        return 0;
+    for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++) {
+        if (!setup_dp(x, sk_DIST_POINT_value(x->crldp, i)))
+            return 0;
+    }
+    return 1;
 }
 
 #define V1_ROOT (EXFLAG_V1|EXFLAG_SS)
@@ -366,12 +374,13 @@ static void x509v3_cache_extensions(X509 *x)
         return;
     }
 
-    X509_digest(x, EVP_sha1(), x->sha1_hash, NULL);
+    if (!X509_digest(x, EVP_sha1(), x->sha1_hash, NULL))
+        x->ex_flags |= EXFLAG_INVALID;
     /* V1 should mean no extensions ... */
     if (!X509_get_version(x))
         x->ex_flags |= EXFLAG_V1;
     /* Handle basic constraints */
-    if ((bs = X509_get_ext_d2i(x, NID_basic_constraints, NULL, NULL))) {
+    if ((bs = X509_get_ext_d2i(x, NID_basic_constraints, &i, NULL))) {
         if (bs->ca)
             x->ex_flags |= EXFLAG_CA;
         if (bs->pathlen) {
@@ -385,9 +394,11 @@ static void x509v3_cache_extensions(X509 *x)
             x->ex_pathlen = -1;
         BASIC_CONSTRAINTS_free(bs);
         x->ex_flags |= EXFLAG_BCONS;
+    } else if (i != -1) {
+        x->ex_flags |= EXFLAG_INVALID;
     }
     /* Handle proxy certificates */
-    if ((pci = X509_get_ext_d2i(x, NID_proxyCertInfo, NULL, NULL))) {
+    if ((pci = X509_get_ext_d2i(x, NID_proxyCertInfo, &i, NULL))) {
         if (x->ex_flags & EXFLAG_CA
             || X509_get_ext_by_NID(x, NID_subject_alt_name, -1) >= 0
             || X509_get_ext_by_NID(x, NID_issuer_alt_name, -1) >= 0) {
@@ -399,9 +410,11 @@ static void x509v3_cache_extensions(X509 *x)
             x->ex_pcpathlen = -1;
         PROXY_CERT_INFO_EXTENSION_free(pci);
         x->ex_flags |= EXFLAG_PROXY;
+    } else if (i != -1) {
+        x->ex_flags |= EXFLAG_INVALID;
     }
     /* Handle key usage */
-    if ((usage = X509_get_ext_d2i(x, NID_key_usage, NULL, NULL))) {
+    if ((usage = X509_get_ext_d2i(x, NID_key_usage, &i, NULL))) {
         if (usage->length > 0) {
             x->ex_kusage = usage->data[0];
             if (usage->length > 1)
@@ -410,9 +423,11 @@ static void x509v3_cache_extensions(X509 *x)
             x->ex_kusage = 0;
         x->ex_flags |= EXFLAG_KUSAGE;
         ASN1_BIT_STRING_free(usage);
+    } else if (i != -1) {
+        x->ex_flags |= EXFLAG_INVALID;
     }
     x->ex_xkusage = 0;
-    if ((extusage = X509_get_ext_d2i(x, NID_ext_key_usage, NULL, NULL))) {
+    if ((extusage = X509_get_ext_d2i(x, NID_ext_key_usage, &i, NULL))) {
         x->ex_flags |= EXFLAG_XKUSAGE;
         for (i = 0; i < sk_ASN1_OBJECT_num(extusage); i++) {
             switch (OBJ_obj2nid(sk_ASN1_OBJECT_value(extusage, i))) {
@@ -455,18 +470,26 @@ static void x509v3_cache_extensions(X509 *x)
             }
         }
         sk_ASN1_OBJECT_pop_free(extusage, ASN1_OBJECT_free);
+    } else if (i != -1) {
+        x->ex_flags |= EXFLAG_INVALID;
     }
 
-    if ((ns = X509_get_ext_d2i(x, NID_netscape_cert_type, NULL, NULL))) {
+    if ((ns = X509_get_ext_d2i(x, NID_netscape_cert_type, &i, NULL))) {
         if (ns->length > 0)
             x->ex_nscert = ns->data[0];
         else
             x->ex_nscert = 0;
         x->ex_flags |= EXFLAG_NSCERT;
         ASN1_BIT_STRING_free(ns);
+    } else if (i != -1) {
+        x->ex_flags |= EXFLAG_INVALID;
     }
-    x->skid = X509_get_ext_d2i(x, NID_subject_key_identifier, NULL, NULL);
-    x->akid = X509_get_ext_d2i(x, NID_authority_key_identifier, NULL, NULL);
+    x->skid = X509_get_ext_d2i(x, NID_subject_key_identifier, &i, NULL);
+    if (x->skid == NULL && i != -1)
+        x->ex_flags |= EXFLAG_INVALID;
+    x->akid = X509_get_ext_d2i(x, NID_authority_key_identifier, &i, NULL);
+    if (x->akid == NULL && i != -1)
+        x->ex_flags |= EXFLAG_INVALID;
     /* Does subject name match issuer ? */
     if (!X509_NAME_cmp(X509_get_subject_name(x), X509_get_issuer_name(x))) {
         x->ex_flags |= EXFLAG_SI;
@@ -475,16 +498,22 @@ static void x509v3_cache_extensions(X509 *x)
             !ku_reject(x, KU_KEY_CERT_SIGN))
             x->ex_flags |= EXFLAG_SS;
     }
-    x->altname = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
+    x->altname = X509_get_ext_d2i(x, NID_subject_alt_name, &i, NULL);
+    if (x->altname == NULL && i != -1)
+        x->ex_flags |= EXFLAG_INVALID;
     x->nc = X509_get_ext_d2i(x, NID_name_constraints, &i, NULL);
-    if (!x->nc && (i != -1))
+    if (x->nc == NULL && i != -1)
+        x->ex_flags |= EXFLAG_INVALID;
+    if (!setup_crldp(x))
         x->ex_flags |= EXFLAG_INVALID;
-    setup_crldp(x);
 
 #ifndef OPENSSL_NO_RFC3779
-    x->rfc3779_addr = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, NULL, NULL);
-    x->rfc3779_asid = X509_get_ext_d2i(x, NID_sbgp_autonomousSysNum,
-                                       NULL, NULL);
+    x->rfc3779_addr = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, &i, NULL);
+    if (x->rfc3779_addr == NULL && i != -1)
+        x->ex_flags |= EXFLAG_INVALID;
+    x->rfc3779_asid = X509_get_ext_d2i(x, NID_sbgp_autonomousSysNum, &i, NULL);
+    if (x->rfc3779_asid == NULL && i != -1)
+        x->ex_flags |= EXFLAG_INVALID;
 #endif
     for (i = 0; i < X509_get_ext_count(x); i++) {
         ex = X509_get_ext(x, i);
@@ -778,7 +807,11 @@ int X509_check_issued(X509 *issuer, X509 *subject)
         return X509_V_ERR_SUBJECT_ISSUER_MISMATCH;
 
     x509v3_cache_extensions(issuer);
+    if (issuer->ex_flags & EXFLAG_INVALID)
+        return X509_V_ERR_UNSPECIFIED;
     x509v3_cache_extensions(subject);
+    if (subject->ex_flags & EXFLAG_INVALID)
+        return X509_V_ERR_UNSPECIFIED;
 
     if (subject->akid) {
         int ret = X509_check_akid(issuer, subject->akid);
@@ -861,7 +894,8 @@ uint32_t X509_get_extension_flags(X509 *x)
 uint32_t X509_get_key_usage(X509 *x)
 {
     /* Call for side-effect of computing hash and caching extensions */
-    X509_check_purpose(x, -1, -1);
+    if (X509_check_purpose(x, -1, -1) != 1)
+        return 0;
     if (x->ex_flags & EXFLAG_KUSAGE)
         return x->ex_kusage;
     return UINT32_MAX;
@@ -870,7 +904,8 @@ uint32_t X509_get_key_usage(X509 *x)
 uint32_t X509_get_extended_key_usage(X509 *x)
 {
     /* Call for side-effect of computing hash and caching extensions */
-    X509_check_purpose(x, -1, -1);
+    if (X509_check_purpose(x, -1, -1) != 1)
+        return 0;
     if (x->ex_flags & EXFLAG_XKUSAGE)
         return x->ex_xkusage;
     return UINT32_MAX;
@@ -879,28 +914,32 @@ uint32_t X509_get_extended_key_usage(X509 *x)
 const ASN1_OCTET_STRING *X509_get0_subject_key_id(X509 *x)
 {
     /* Call for side-effect of computing hash and caching extensions */
-    X509_check_purpose(x, -1, -1);
+    if (X509_check_purpose(x, -1, -1) != 1)
+        return NULL;
     return x->skid;
 }
 
 const ASN1_OCTET_STRING *X509_get0_authority_key_id(X509 *x)
 {
     /* Call for side-effect of computing hash and caching extensions */
-    X509_check_purpose(x, -1, -1);
+    if (X509_check_purpose(x, -1, -1) != 1)
+        return NULL;
     return (x->akid != NULL ? x->akid->keyid : NULL);
 }
 
 const GENERAL_NAMES *X509_get0_authority_issuer(X509 *x)
 {
     /* Call for side-effect of computing hash and caching extensions */
-    X509_check_purpose(x, -1, -1);
+    if (X509_check_purpose(x, -1, -1) != 1)
+        return NULL;
     return (x->akid != NULL ? x->akid->issuer : NULL);
 }
 
 const ASN1_INTEGER *X509_get0_authority_serial(X509 *x)
 {
     /* Call for side-effect of computing hash and caching extensions */
-    X509_check_purpose(x, -1, -1);
+    if (X509_check_purpose(x, -1, -1) != 1)
+        return NULL;
     return (x->akid != NULL ? x->akid->serial : NULL);
 }