Hi, On Sun, Dec 23, 2012 at 01:11:22PM +0100, Arne Schwabe wrote: > I fear we may have to delay the final release. While debugging a problem > with a user of my app I found a nasty bug which causes OpenVPN to crash > if two PUSH_CONTROL messages are received.
The appended patch fixes this for me (Client on Linux/i386, but that
should be OS and architecture agnostic).
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
From f38ff2ee809f85b6a7c1dc08bb8cf9bfc350070f Mon Sep 17 00:00:00 2001
From: Gert Doering <g...@greenie.muc.de>
List-Post: openvpn-devel@lists.sourceforge.net
Date: Tue, 25 Dec 2012 13:41:50 +0100
Subject: [PATCH] Fix client crash on double PUSH_REPLY.
Introduce an extra bool variable c2.pulled_options_md5_init_done to
keep track of md5_init state of pulled_options_state - avoid accessing
uninitialized state when a second PUSH_REPLY comes in (which only happens
under very particular circumstances).
Bug tracked down by Arne Schwabe <a...@rfc2549.rrg>.
Signed-off-by: Gert Doering <g...@greenie.muc.de>
---
src/openvpn/openvpn.h | 1 +
src/openvpn/push.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 7abfb08..bdfa685 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -474,6 +474,7 @@ struct context_2
bool did_pre_pull_restore;
/* hash of pulled options, so we can compare when options change */
+ bool pulled_options_md5_init_done;
struct md5_state pulled_options_state;
struct md5_digest pulled_options_digest;
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 05a38e0..be50bef 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -446,10 +446,14 @@ process_incoming_push_msg (struct context *c,
if (ch == ',')
{
struct buffer buf_orig = buf;
+ if (!c->c2.pulled_options_md5_init_done)
+ {
+ md5_state_init (&c->c2.pulled_options_state);
+ c->c2.pulled_options_md5_init_done = true;
+ }
if (!c->c2.did_pre_pull_restore)
{
pre_pull_restore (&c->options);
- md5_state_init (&c->c2.pulled_options_state);
c->c2.did_pre_pull_restore = true;
}
if (apply_push_options (&c->options,
@@ -463,6 +467,7 @@ process_incoming_push_msg (struct context *c,
case 1:
md5_state_update (&c->c2.pulled_options_state, BPTR(&buf_orig),
BLEN(&buf_orig));
md5_state_final (&c->c2.pulled_options_state,
&c->c2.pulled_options_digest);
+ c->c2.pulled_options_md5_init_done = false;
ret = PUSH_MSG_REPLY;
break;
case 2:
--
1.7.8.6
pgpEbcjTLJwfU.pgp
Description: PGP signature
