From b1cc84e82d41ab669bf804ea519f5332c48a3d77 Mon Sep 17 00:00:00 2001 From: Clemens Lang Date: Wed, 24 May 2023 12:22:25 +0200 Subject: [PATCH] x509: Fix possible use-after-free when OOM ossl_policy_level_add_node() first adds the new node to the level->nodes stack, and then attempts to add extra data if extra_data is true. If memory allocation or adding the extra data to tree->extra_data fails, the allocated node (that has already been added to the level->nodes stack) is freed using ossl_policy_node_free(), which leads to a potential use after free. Additionally, the tree's node count and the parent's child count would not be updated, despite the new node being added. Fix this by either performing the function's purpose completely, or not at all by reverting the changes on error. Signed-off-by: Clemens Lang Reviewed-by: Dmitry Belyavskiy Reviewed-by: Matt Caswell Reviewed-by: Bernd Edlinger Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/21066) --- crypto/x509v3/pcy_node.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crypto/x509v3/pcy_node.c b/crypto/x509v3/pcy_node.c index d574fb9d665..c6c01cbb395 100644 --- a/crypto/x509v3/pcy_node.c +++ b/crypto/x509v3/pcy_node.c @@ -100,11 +100,11 @@ X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level, tree->extra_data = sk_X509_POLICY_DATA_new_null(); if (tree->extra_data == NULL){ X509V3err(X509V3_F_LEVEL_ADD_NODE, ERR_R_MALLOC_FAILURE); - goto node_error; + goto extra_data_error; } if (!sk_X509_POLICY_DATA_push(tree->extra_data, data)) { X509V3err(X509V3_F_LEVEL_ADD_NODE, ERR_R_MALLOC_FAILURE); - goto node_error; + goto extra_data_error; } } @@ -114,6 +114,14 @@ X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level, return node; + extra_data_error: + if (level != NULL) { + if (level->anyPolicy == node) + level->anyPolicy = NULL; + else + (void) sk_X509_POLICY_NODE_pop(level->nodes); + } + node_error: policy_node_free(node); return NULL; -- 2.47.2