diff options
| author | tb <> | 2020-11-18 17:40:42 +0000 |
|---|---|---|
| committer | tb <> | 2020-11-18 17:40:42 +0000 |
| commit | c67f3c9390fc29f3f4e97751c76c023f647bc9ec (patch) | |
| tree | 5cfa5758bf11e0b6fd7af75ce23fb2828bf6879e /src/lib | |
| parent | 39533a204dedf2e29d621dfe9a9f4e24657fdafd (diff) | |
| download | openbsd-c67f3c9390fc29f3f4e97751c76c023f647bc9ec.tar.gz openbsd-c67f3c9390fc29f3f4e97751c76c023f647bc9ec.tar.bz2 openbsd-c67f3c9390fc29f3f4e97751c76c023f647bc9ec.zip | |
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
Diffstat (limited to 'src/lib')
| -rw-r--r-- | src/lib/libcrypto/x509/x509_vfy.c | 7 |
1 files changed, 6 insertions, 1 deletions
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 @@ | |||
| 1 | /* $OpenBSD: x509_vfy.c,v 1.83 2020/11/18 17:08:59 tb Exp $ */ | 1 | /* $OpenBSD: x509_vfy.c,v 1.84 2020/11/18 17:40:42 tb Exp $ */ |
| 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) | 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * | 4 | * |
| @@ -1794,6 +1794,11 @@ x509_vfy_check_policy(X509_STORE_CTX *ctx) | |||
| 1794 | 1794 | ||
| 1795 | if (ctx->parent) | 1795 | if (ctx->parent) |
| 1796 | return 1; | 1796 | return 1; |
| 1797 | |||
| 1798 | /* X509_policy_check always allocates a new tree. */ | ||
| 1799 | X509_policy_tree_free(ctx->tree); | ||
| 1800 | ctx->tree = NULL; | ||
| 1801 | |||
| 1797 | ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain, | 1802 | ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain, |
| 1798 | ctx->param->policies, ctx->param->flags); | 1803 | ctx->param->policies, ctx->param->flags); |
| 1799 | if (ret == 0) { | 1804 | if (ret == 0) { |