From c67f3c9390fc29f3f4e97751c76c023f647bc9ec Mon Sep 17 00:00:00 2001 From: tb <> Date: Wed, 18 Nov 2020 17:40:42 +0000 Subject: Plug a big memory leak in the new validator The legacy validator would only call x509_vfy_check_policy() once at the very end after cobbling together a chain. Therefore it didn't matter that X509_policy_check() always allocates a new tree on top of the one that might have been passed in. This is in stark contrast to other, similar APIs in this code base. The new validator calls this function several times over while building its chains. This adds up to a sizable leak in the new validator. Reported with a reproducer by Hanno Zysik on github, who also bisected this to the commit enabling the new validator. Narrowed down to x509_vfy_check_policy() by jsing. We simultaenously came up with a functionally identical fix. ok jsing --- src/lib/libcrypto/x509/x509_vfy.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/libcrypto/x509/x509_vfy.c b/src/lib/libcrypto/x509/x509_vfy.c index 0bd41295d9..7daf8bf60e 100644 --- a/src/lib/libcrypto/x509/x509_vfy.c +++ b/src/lib/libcrypto/x509/x509_vfy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.c,v 1.83 2020/11/18 17:08:59 tb Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.84 2020/11/18 17:40:42 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1794,6 +1794,11 @@ x509_vfy_check_policy(X509_STORE_CTX *ctx) if (ctx->parent) return 1; + + /* X509_policy_check always allocates a new tree. */ + X509_policy_tree_free(ctx->tree); + ctx->tree = NULL; + ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain, ctx->param->policies, ctx->param->flags); if (ret == 0) { -- cgit v1.2.3-55-g6feb