Hi, On 13-05-17 15:40, Antonio Quartulli wrote: >> On 13 May 2017, at 18:37, Guido Vranken <guidovran...@gmail.com> wrote: >> >> Signed-off-by: Guido Vranken <guidovran...@gmail.com> >> --- >> src/openvpn/ssl_verify_openssl.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/openvpn/ssl_verify_openssl.c >> b/src/openvpn/ssl_verify_openssl.c >> index 8374783..d64f83c 100644 >> --- a/src/openvpn/ssl_verify_openssl.c >> +++ b/src/openvpn/ssl_verify_openssl.c >> @@ -285,11 +285,11 @@ x509_get_subject (X509 *cert, struct gc_arena *gc) >> >> BIO_get_mem_ptr (subject_bio, &subject_mem); > > I don’t think you patch is based on master, isn’t it? > The codestyle has been recently revisited and there should be no invocation > of a function with a space between its name and the ‘(‘. So this code looks > old. > >> >> - maxlen = subject_mem->length + 1; >> - subject = gc_malloc (maxlen, false, gc); >> + maxlen = subject_mem->length; >> + subject = gc_malloc (maxlen+1, false, gc); > > > Please keep the spaces around arithmetical operators (i.e. “maxlen + 1”); > > >> >> memcpy (subject, subject_mem->data, maxlen); > > wouldn’t the patch just be way easier if you had changed “maxlen” with > “maxlen - 1” in the line above? This way you wouldn’t need to change the rest > and maxlen would still be the real maxlen. > > But this is just my personal taste.
This patch indeed seems to be against the release/2.3 branch. I took the liberty to 'forward port' the change to master, see the attachment. As for style, I don't care too much about release/2.3, it's a style mess anyway... Seeing that this fixes a real (though not exposed) bug, ACK to Guido's patch for release/2.3. -Steffan
From 6b26a069052898cca1a1511527857df448a2dd49 Mon Sep 17 00:00:00 2001
From: Steffan Karger <stef...@karger.me>
Date: Sun, 14 May 2017 21:00:41 +0200
Subject: [PATCH] Avoid a 1 byte overcopy in x509_get_subject
(ssl_verify_openssl.c)
This is the equivalent of the patch sent earlier by Guide Vranken, but
adjusted to code in the master and release/2.4 branches.
Signed-off-by: Steffan Karger <stef...@karger.me>
---
src/openvpn/ssl_verify_openssl.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index d7202b6..1079759 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -317,7 +317,6 @@ x509_get_subject(X509 *cert, struct gc_arena *gc)
BIO *subject_bio = NULL;
BUF_MEM *subject_mem;
char *subject = NULL;
- int maxlen = 0;
/*
* Generate the subject string in OpenSSL proprietary format,
@@ -348,11 +347,10 @@ x509_get_subject(X509 *cert, struct gc_arena *gc)
BIO_get_mem_ptr(subject_bio, &subject_mem);
- maxlen = subject_mem->length + 1;
- subject = gc_malloc(maxlen, false, gc);
+ subject = gc_malloc(subject_mem->length + 1, false, gc);
- memcpy(subject, subject_mem->data, maxlen);
- subject[maxlen - 1] = '\0';
+ memcpy(subject, subject_mem->data, subject_mem->length);
+ subject[subject_mem->length] = '\0';
err:
if (subject_bio)
--
2.7.4
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
