Message ID | 20210603123019.422644-1-matthias.andree@gmx.de |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] Fix SIGSEGV (NULL deref) receiving push "echo" | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> This is obviously correct, and in hindsight a very obvious bug - we seemed to have focused too much on the documentation aspect of the commit introducing the bug, and not enough on the code change... Your patch has been applied to the master and release/2.5 branch. commit 0033811e0215af76f469d78912c95a2f59813454 (master). commit d6e21bed964109abaf4bf03a951dc2fc9b1d5c1f (release/2.5) Author: Matthias Andree Date: Thu Jun 3 14:30:19 2021 +0200 Fix SIGSEGV (NULL deref) receiving push echo Signed-off-by: Matthias Andree <matthias.andree@gmx.de> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20210603123019.422644-1-matthias.andree@gmx.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22486.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
On Thu, Jun 3, 2021 at 8:32 AM Matthias Andree <matthias.andree@gmx.de> wrote: > > A server pushing "echo" without arguments can crash the client. > In such a situation, the code in question receives p[1] == NULL > (which was CLEAR(p)'ed above), hands it strncmp, which then > dereferences the null pointer. > > Original report and analysis here: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256331 > > Fixes: Trac #1409 > Reported-by: peo@nethead.se (to FreeBSD) > Signed-off-by: Matthias Andree <matthias.andree@gmx.de> > --- > src/openvpn/options.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index 8d417206..a54bc562 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -5365,7 +5365,7 @@ add_option(struct options *options, > { > /* only message-related ECHO are logged, since other ECHOs > * can potentially include security-sensitive strings */ > - if (strncmp(p[1], "msg", 3) == 0) > + if (p[1] && strncmp(p[1], "msg", 3) == 0) > { > msg(M_INFO, "%s:%s", > pull_mode ? "ECHO-PULL" : "ECHO", Argh.. good to see already committed. I take all the blame for acking the original.. This may take the award for the easiest bug to reproduce: just run "openvpn --echo" no server, no config file required. Selva
diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 8d417206..a54bc562 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5365,7 +5365,7 @@ add_option(struct options *options, { /* only message-related ECHO are logged, since other ECHOs * can potentially include security-sensitive strings */ - if (strncmp(p[1], "msg", 3) == 0) + if (p[1] && strncmp(p[1], "msg", 3) == 0) { msg(M_INFO, "%s:%s", pull_mode ? "ECHO-PULL" : "ECHO",
A server pushing "echo" without arguments can crash the client. In such a situation, the code in question receives p[1] == NULL (which was CLEAR(p)'ed above), hands it strncmp, which then dereferences the null pointer. Original report and analysis here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256331 Fixes: Trac #1409 Reported-by: peo@nethead.se (to FreeBSD) Signed-off-by: Matthias Andree <matthias.andree@gmx.de> --- src/openvpn/options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.31.1