From 5cea5841c70be0186c11ff79a9767d2e1376e80a Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 17 May 2020 14:45:28 +0200 Subject: [PATCH] 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 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/11852) --- crypto/err/openssl.txt | 1 + crypto/x509/x509_err.c | 3 ++- crypto/x509/x509_vfy.c | 52 ++++++++++++++++++++++++++++----------- crypto/x509/x_pubkey.c | 7 ++++-- include/openssl/x509err.h | 7 +++--- 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 -# endif +# include # 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 -- 2.25.1