Fix some places where X509_up_ref is used

without error handling.

This takes up the ball from #11278
without trying to solve everything at once.

[extended tests]

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11852)

5 files changed, 49 insertions, 21 deletions

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 35512f9caf..c90df98c29 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -1742,6 +1742,7 @@ X509_F_X509_NAME_PRINT:117:X509_NAME_print
 X509_F_X509_OBJECT_NEW:150:X509_OBJECT_new
 X509_F_X509_PRINT_EX_FP:118:X509_print_ex_fp
 X509_F_X509_PUBKEY_DECODE:148:x509_pubkey_decode
+X509_F_X509_PUBKEY_GET:161:X509_PUBKEY_get
 X509_F_X509_PUBKEY_GET0:119:X509_PUBKEY_get0
 X509_F_X509_PUBKEY_SET:120:X509_PUBKEY_set
 X509_F_X509_REQ_CHECK_PRIVATE_KEY:144:X509_REQ_check_private_key
diff --git a/crypto/x509/x509_err.c b/crypto/x509/x509_err.c
index c110d90809..bdd1e67cd3 100644
--- a/crypto/x509/x509_err.c
+++ b/crypto/x509/x509_err.c
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -79,6 +79,7 @@ static const ERR_STRING_DATA X509_str_functs[] = {
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_PRINT_EX_FP, 0), "X509_print_ex_fp"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_PUBKEY_DECODE, 0),
      "x509_pubkey_decode"},
+    {ERR_PACK(ERR_LIB_X509, X509_F_X509_PUBKEY_GET, 0), "X509_PUBKEY_get"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_PUBKEY_GET0, 0), "X509_PUBKEY_get0"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_PUBKEY_SET, 0), "X509_PUBKEY_set"},
     {ERR_PACK(ERR_LIB_X509, X509_F_X509_REQ_CHECK_PRIVATE_KEY, 0),
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 41625e75ad..39e0c53de0 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -131,10 +131,9 @@ static X509 *lookup_cert_match(X509_STORE_CTX *ctx, X509 *x)
         xtmp = sk_X509_value(certs, i);
         if (!X509_cmp(xtmp, x))
             break;
+        xtmp = NULL;
     }
-    if (i < sk_X509_num(certs))
-        X509_up_ref(xtmp);
-    else
+    if (xtmp != NULL && !X509_up_ref(xtmp))
         xtmp = NULL;
     sk_X509_pop_free(certs, X509_free);
     return xtmp;
@@ -267,17 +266,24 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
         return -1;
     }
 
+    if (!X509_up_ref(ctx->cert)) {
+        X509err(X509_F_X509_VERIFY_CERT, ERR_R_INTERNAL_ERROR);
+        ctx->error = X509_V_ERR_UNSPECIFIED;
+        return -1;
+    }
+
     /*
      * first we make sure the chain we are going to build is present and that
      * the first entry is in place
      */
-    if (((ctx->chain = sk_X509_new_null()) == NULL) ||
-        (!sk_X509_push(ctx->chain, ctx->cert))) {
+    if ((ctx->chain = sk_X509_new_null()) == NULL
+            || !sk_X509_push(ctx->chain, ctx->cert)) {
+        X509_free(ctx->cert);
         X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
         ctx->error = X509_V_ERR_OUT_OF_MEM;
         return -1;
     }
-    X509_up_ref(ctx->cert);
+
     ctx->num_untrusted = 1;
 
     /* If the peer's public key is too weak, we can stop early. */
@@ -350,11 +356,15 @@ static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer)
 static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
 {
     *issuer = find_issuer(ctx, ctx->other_ctx, x);
-    if (*issuer) {
-        X509_up_ref(*issuer);
-        return 1;
-    } else
-        return 0;
+
+    if (*issuer == NULL || !X509_up_ref(*issuer))
+        goto err;
+
+    return 1;
+
+ err:
+    *issuer = NULL;
+    return 0;
 }
 
 static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx, X509_NAME *nm)
@@ -366,15 +376,20 @@ static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx, X509_NAME *nm)
     for (i = 0; i < sk_X509_num(ctx->other_ctx); i++) {
         x = sk_X509_value(ctx->other_ctx, i);
         if (X509_NAME_cmp(nm, X509_get_subject_name(x)) == 0) {
+            if (!X509_up_ref(x)) {
+                X509err(X509_F_LOOKUP_CERTS_SK, ERR_R_INTERNAL_ERROR);
+                ctx->error = X509_V_ERR_UNSPECIFIED;
+                return NULL;
+            }
             if (sk == NULL)
                 sk = sk_X509_new_null();
-            if (sk == NULL || sk_X509_push(sk, x) == 0) {
+            if (sk == NULL || !sk_X509_push(sk, x)) {
+                X509_free(x);
                 sk_X509_pop_free(sk, X509_free);
                 X509err(X509_F_LOOKUP_CERTS_SK, ERR_R_MALLOC_FAILURE);
                 ctx->error = X509_V_ERR_OUT_OF_MEM;
                 return NULL;
             }
-            X509_up_ref(x);
         }
     }
     return sk;
@@ -3158,7 +3173,16 @@ static int build_chain(X509_STORE_CTX *ctx)
             /* Drop this issuer from future consideration */
             (void) sk_X509_delete_ptr(sktmp, xtmp);
 
+            if (!X509_up_ref(xtmp)) {
+                X509err(X509_F_BUILD_CHAIN, ERR_R_INTERNAL_ERROR);
+                trust = X509_TRUST_REJECTED;
+                ctx->error = X509_V_ERR_UNSPECIFIED;
+                search = 0;
+                continue;
+            }
+
             if (!sk_X509_push(ctx->chain, xtmp)) {
+                X509_free(xtmp);
                 X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);
                 trust = X509_TRUST_REJECTED;
                 ctx->error = X509_V_ERR_OUT_OF_MEM;
@@ -3166,7 +3190,7 @@ static int build_chain(X509_STORE_CTX *ctx)
                 continue;
             }
 
-            X509_up_ref(x = xtmp);
+            x = xtmp;
             ++ctx->num_untrusted;
             ss = cert_self_signed(xtmp);
 
diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c
index 4f694b93fb..f175097094 100644
--- a/crypto/x509/x_pubkey.c
+++ b/crypto/x509/x_pubkey.c
@@ -169,8 +169,11 @@ EVP_PKEY *X509_PUBKEY_get0(X509_PUBKEY *key)
 EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key)
 {
     EVP_PKEY *ret = X509_PUBKEY_get0(key);
-    if (ret != NULL)
-        EVP_PKEY_up_ref(ret);
+
+    if (ret != NULL && !EVP_PKEY_up_ref(ret)) {
+        X509err(X509_F_X509_PUBKEY_GET, ERR_R_INTERNAL_ERROR);
+        ret = NULL;
+    }
     return ret;
 }
 
diff --git a/include/openssl/x509err.h b/include/openssl/x509err.h
index 0273853172..cd08673f8f 100644
--- a/include/openssl/x509err.h
+++ b/include/openssl/x509err.h
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -11,9 +11,7 @@
 #ifndef HEADER_X509ERR_H
 # define HEADER_X509ERR_H
 
-# ifndef HEADER_SYMHACKS_H
-#  include <openssl/symhacks.h>
-# endif
+# include <openssl/symhacks.h>
 
 # ifdef  __cplusplus
 extern "C"
@@ -65,6 +63,7 @@ int ERR_load_X509_strings(void);
 # define X509_F_X509_OBJECT_NEW                           150
 # define X509_F_X509_PRINT_EX_FP                          118
 # define X509_F_X509_PUBKEY_DECODE                        148
+# define X509_F_X509_PUBKEY_GET                           161
 # define X509_F_X509_PUBKEY_GET0                          119
 # define X509_F_X509_PUBKEY_SET                           120
 # define X509_F_X509_REQ_CHECK_PRIVATE_KEY                144