|
[Openvpn-devel] [XS] Change in openvpn[master]: mbedtls: gracefully
exit if certificate file is NULL
From: syzzer (C. Review) <ge...@op...> - 2025-12-06 20:51:08
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email
to review the following change.
Change subject: mbedtls: gracefully exit if certificate file is NULL
......................................................................
mbedtls: gracefully exit if certificate file is NULL
Instead of crashing because we feed a NULL pointer to strlen(),
gracefully exit with an error message.
While at it, improve the error message a bit.
Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1
Reported-By: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)
Signed-off-by: Steffan Karger <st...@ka...>
---
M src/openvpn/ssl_mbedtls.c
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1419/1
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 83fca78..3440319 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -466,10 +466,14 @@
if (cert_inline)
{
+ if (!cert_file)
+ {
+ msg(M_FATAL, "Cannot load inline certificate: NULL");
+ }
if (!mbed_ok(mbedtls_x509_crt_parse(ctx->crt_chain, (const unsigned char *)cert_file,
strlen(cert_file) + 1)))
{
- msg(M_FATAL, "Cannot load inline certificate file");
+ msg(M_FATAL, "Cannot load inline certificate");
}
}
else
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1
Gerrit-Change-Number: 1419
Gerrit-PatchSet: 1
Gerrit-Owner: syzzer <st...@ka...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
[Openvpn-devel] [XS] Change in openvpn[master]: mbedtls: gracefully
exit if certificate file is NULL
From: cron2 (C. Review) <ge...@op...> - 2025-12-06 20:58:19
|
Attention is currently required from: plaisthos, syzzer. cron2 has posted comments on this change by syzzer. ( http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email ) Change subject: mbedtls: gracefully exit if certificate file is NULL ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: need to test a bit how to trigger this :-) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1 Gerrit-Change-Number: 1419 Gerrit-PatchSet: 1 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: syzzer <st...@ka...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Sat, 06 Dec 2025 20:58:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-12-06 20:58:40
|
From: Steffan Karger <st...@ka...> Instead of crashing because we feed a NULL pointer to strlen(), gracefully exit with an error message. While at it, improve the error message a bit. Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1 Reported-By: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1419 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1419 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 83fca78..3440319 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -466,10 +466,14 @@ if (cert_inline) { + if (!cert_file) + { + msg(M_FATAL, "Cannot load inline certificate: NULL"); + } if (!mbed_ok(mbedtls_x509_crt_parse(ctx->crt_chain, (const unsigned char *)cert_file, strlen(cert_file) + 1))) { - msg(M_FATAL, "Cannot load inline certificate file"); + msg(M_FATAL, "Cannot load inline certificate"); } } else |
|
From: Gert D. <ge...@gr...> - 2025-12-06 22:38:20
|
I have not tested if I can reproduce the situation (like with an empty
inline <cert></cert> cert, or somehow via management interface) or if
this is caught further upstream - but this check looks quite reasonable,
and the BBs are fine with it.
Your patch has been applied to the master branch.
commit d7c7caa370ad1fff1cd222e2499a77ea792c8a0e
Author: Steffan Karger
Date: Sat Dec 6 21:58:16 2025 +0100
mbedtls: gracefully exit if certificate file is NULL
Signed-off-by: Steffan Karger <st...@ka...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1419
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34864.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
[Openvpn-devel] [XS] Change in openvpn[master]: mbedtls: gracefully
exit if certificate file is NULL
From: cron2 (C. Review) <ge...@op...> - 2025-12-06 22:38:55
|
cron2 has uploaded a new patch set (#2) to the change originally created by syzzer. ( http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: mbedtls: gracefully exit if certificate file is NULL ...................................................................... mbedtls: gracefully exit if certificate file is NULL Instead of crashing because we feed a NULL pointer to strlen(), gracefully exit with an error message. While at it, improve the error message a bit. Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1 Reported-By: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1419 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34864.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_mbedtls.c 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1419/2 diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 83fca78..3440319 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -466,10 +466,14 @@ if (cert_inline) { + if (!cert_file) + { + msg(M_FATAL, "Cannot load inline certificate: NULL"); + } if (!mbed_ok(mbedtls_x509_crt_parse(ctx->crt_chain, (const unsigned char *)cert_file, strlen(cert_file) + 1))) { - msg(M_FATAL, "Cannot load inline certificate file"); + msg(M_FATAL, "Cannot load inline certificate"); } } else -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1 Gerrit-Change-Number: 1419 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
[Openvpn-devel] [XS] Change in openvpn[master]: mbedtls: gracefully
exit if certificate file is NULL
From: cron2 (C. Review) <ge...@op...> - 2025-12-06 22:39:02
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email ) Change subject: mbedtls: gracefully exit if certificate file is NULL ...................................................................... mbedtls: gracefully exit if certificate file is NULL Instead of crashing because we feed a NULL pointer to strlen(), gracefully exit with an error message. While at it, improve the error message a bit. Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1 Reported-By: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1419 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34864.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_mbedtls.c 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 83fca78..3440319 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -466,10 +466,14 @@ if (cert_inline) { + if (!cert_file) + { + msg(M_FATAL, "Cannot load inline certificate: NULL"); + } if (!mbed_ok(mbedtls_x509_crt_parse(ctx->crt_chain, (const unsigned char *)cert_file, strlen(cert_file) + 1))) { - msg(M_FATAL, "Cannot load inline certificate file"); + msg(M_FATAL, "Cannot load inline certificate"); } } else -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1419?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I0d592d9d0c7ead296869f933c206c5d55e6cbed1 Gerrit-Change-Number: 1419 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |