From 42f3108a7d2860fcc392d6a69a9ad058abd50338 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 12 Mar 2021 15:53:38 +0000 Subject: [PATCH] Fix checks of memory caps of constraints names x509_internal.h defines caps on the number of name constraints and other names (such as subjectAltNames) that we want to allocate per cert chain. These limits are checked too late. In a particularly silly cert that jan found on ugos.ugm.ac.id 443, we ended up allocating six times 2048 x509_constraint_name structures before deciding that these are more than 512. Fix this by adding a names_max member to x509_constraints_names which is set on allocation against which each addition of a name is checked. cluebat/ok jsing ok inoguchi on earlier version --- lib/libcrypto/x509/x509_constraints.c | 34 +++++++++++++++++---------- lib/libcrypto/x509/x509_internal.h | 7 +++--- lib/libcrypto/x509/x509_verify.c | 11 +++++---- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c index 67cbaa63137..2d55e136d73 100644 --- a/lib/libcrypto/x509/x509_constraints.c +++ b/lib/libcrypto/x509/x509_constraints.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_constraints.c,v 1.12 2020/11/25 21:17:52 tb Exp $ */ +/* $OpenBSD: x509_constraints.c,v 1.13 2021/03/12 15:53:38 tb Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -86,9 +86,16 @@ x509_constraints_name_dup(struct x509_constraints_name *name) } struct x509_constraints_names * -x509_constraints_names_new() +x509_constraints_names_new(size_t names_max) { - return (calloc(1, sizeof(struct x509_constraints_names))); + struct x509_constraints_names *new; + + if ((new = calloc(1, sizeof(struct x509_constraints_names))) == NULL) + return NULL; + + new->names_max = names_max; + + return new; } void @@ -118,6 +125,8 @@ x509_constraints_names_add(struct x509_constraints_names *names, { size_t i = names->names_count; + if (names->names_count >= names->names_max) + return 0; if (names->names_count == names->names_len) { struct x509_constraints_name **tmp; if ((tmp = recallocarray(names->names, names->names_len, @@ -141,14 +150,16 @@ x509_constraints_names_dup(struct x509_constraints_names *names) if (names == NULL) return NULL; - if ((new = x509_constraints_names_new()) == NULL) + if ((new = x509_constraints_names_new(names->names_max)) == NULL) goto err; + for (i = 0; i < names->names_count; i++) { if ((name = x509_constraints_name_dup(names->names[i])) == NULL) goto err; if (!x509_constraints_names_add(new, name)) goto err; } + return new; err: x509_constraints_names_free(new); @@ -1117,7 +1128,8 @@ x509_constraints_chain(STACK_OF(X509) *chain, int *error, int *depth) goto err; if (chain_length == 1) return 1; - if ((names = x509_constraints_names_new()) == NULL) { + if ((names = x509_constraints_names_new( + X509_VERIFY_MAX_CHAIN_NAMES)) == NULL) { verify_err = X509_V_ERR_OUT_OF_MEM; goto err; } @@ -1130,13 +1142,13 @@ x509_constraints_chain(STACK_OF(X509) *chain, int *error, int *depth) if ((cert = sk_X509_value(chain, i)) == NULL) goto err; if (cert->nc != NULL) { - if ((permitted = - x509_constraints_names_new()) == NULL) { + if ((permitted = x509_constraints_names_new( + X509_VERIFY_MAX_CHAIN_CONSTRAINTS)) == NULL) { verify_err = X509_V_ERR_OUT_OF_MEM; goto err; } - if ((excluded = - x509_constraints_names_new()) == NULL) { + if ((excluded = x509_constraints_names_new( + X509_VERIFY_MAX_CHAIN_CONSTRAINTS)) == NULL) { verify_err = X509_V_ERR_OUT_OF_MEM; goto err; } @@ -1161,10 +1173,6 @@ x509_constraints_chain(STACK_OF(X509) *chain, int *error, int *depth) if (!x509_constraints_extract_names(names, cert, 0, &verify_err)) goto err; - if (names->names_count > X509_VERIFY_MAX_CHAIN_NAMES) { - verify_err = X509_V_ERR_OUT_OF_MEM; - goto err; - } } x509_constraints_names_free(names); diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h index 1ede7b6bad3..fe403512284 100644 --- a/lib/libcrypto/x509/x509_internal.h +++ b/lib/libcrypto/x509/x509_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_internal.h,v 1.6 2021/01/05 16:45:59 jsing Exp $ */ +/* $OpenBSD: x509_internal.h,v 1.7 2021/03/12 15:53:38 tb Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -51,8 +51,9 @@ struct x509_constraints_name { struct x509_constraints_names { struct x509_constraints_name **names; - size_t names_len; size_t names_count; + size_t names_len; + size_t names_max; }; struct x509_verify_chain { @@ -101,7 +102,7 @@ int x509_constraints_names_add(struct x509_constraints_names *names, struct x509_constraints_names *x509_constraints_names_dup( struct x509_constraints_names *names); void x509_constraints_names_clear(struct x509_constraints_names *names); -struct x509_constraints_names *x509_constraints_names_new(void); +struct x509_constraints_names *x509_constraints_names_new(size_t names_max); void x509_constraints_names_free(struct x509_constraints_names *names); int x509_constraints_valid_host(uint8_t *name, size_t len); int x509_constraints_valid_sandns(uint8_t *name, size_t len); diff --git a/lib/libcrypto/x509/x509_verify.c b/lib/libcrypto/x509/x509_verify.c index f51ea1d8689..3c8369f1f9a 100644 --- a/lib/libcrypto/x509/x509_verify.c +++ b/lib/libcrypto/x509/x509_verify.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_verify.c,v 1.34 2021/02/26 15:19:41 tb Exp $ */ +/* $OpenBSD: x509_verify.c,v 1.35 2021/03/12 15:53:38 tb Exp $ */ /* * Copyright (c) 2020-2021 Bob Beck * @@ -52,7 +52,8 @@ x509_verify_chain_new(void) if ((chain->cert_errors = calloc(X509_VERIFY_MAX_CHAIN_CERTS, sizeof(int))) == NULL) goto err; - if ((chain->names = x509_constraints_names_new()) == NULL) + if ((chain->names = + x509_constraints_names_new(X509_VERIFY_MAX_CHAIN_NAMES)) == NULL) goto err; return chain; @@ -720,11 +721,13 @@ x509_verify_validate_constraints(X509 *cert, return 1; if (cert->nc != NULL) { - if ((permitted = x509_constraints_names_new()) == NULL) { + if ((permitted = x509_constraints_names_new( + X509_VERIFY_MAX_CHAIN_CONSTRAINTS)) == NULL) { err = X509_V_ERR_OUT_OF_MEM; goto err; } - if ((excluded = x509_constraints_names_new()) == NULL) { + if ((excluded = x509_constraints_names_new( + X509_VERIFY_MAX_CHAIN_CONSTRAINTS)) == NULL) { err = X509_V_ERR_OUT_OF_MEM; goto err; } -- 2.20.1