Security Vulnerability -- An OpenVPN client connecting to a

malicious or compromised server could potentially receive
"setenv" configuration directives from the server which could
cause arbitrary code execution on the client via a LD_PRELOAD
attack.  A successful attack appears to require that (a) the
client has agreed to allow the server to push configuration
directives to it by including "pull" or the macro "client" in
its configuration file, (b) the client configuration file uses
a scripting directive such as "up" or "down", (c) the client
succesfully authenticates the server, (d) the server is
malicious or has been compromised and is under the control of
the attacker, and (e) the attacker has at least some level of
pre-existing control over files on the client (this might be
accomplished by having the server respond to a client web request
with a specially crafted file).

The fix is to disallow "setenv" to be pushed to clients from
the server, and to add a new directive "setenv-safe" which is
pushable from the server, but which appends "OPENVPN_" to the
name of each remotely set environmental variable.


git-svn-id: http://svn.openvpn.net/projects/openvpn/branches/BETA21/openvpn@983 e7ae566f-a301-0410-adde-c780ea21d3b5

3 files changed, 15 insertions, 1 deletions

diff --git a/misc.c b/misc.c
index b7eaae9..0c45817 100644
--- a/misc.c
+++ b/misc.c
@@ -870,6 +870,14 @@ setenv_str (struct env_set *es, const char *name, const char *value)
 }
 
 void
+setenv_str_safe (struct env_set *es, const char *name, const char *value)
+{
+  char buf[64];
+  openvpn_snprintf (buf, sizeof(buf), "OPENVPN_%s", name);
+  setenv_str (es, buf, value);
+}
+
+void
 setenv_del (struct env_set *es, const char *name)
 {
   ASSERT (name);
diff --git a/misc.h b/misc.h
index 4f6e7eb..ae78827 100644
--- a/misc.h
+++ b/misc.h
@@ -161,6 +161,7 @@ void setenv_str_ex (struct env_set *es,
 void setenv_counter (struct env_set *es, const char *name, counter_type value);
 void setenv_int (struct env_set *es, const char *name, int value);
 void setenv_str (struct env_set *es, const char *name, const char *value);
+void setenv_str_safe (struct env_set *es, const char *name, const char *value);
 void setenv_del (struct env_set *es, const char *name);
 
 /* struct env_set functions */
diff --git a/options.c b/options.c
index a6b5ba5..a889eb5 100644
--- a/options.c
+++ b/options.c
@@ -3989,9 +3989,14 @@ add_option (struct options *options,
     }
   else if (streq (p[0], "setenv") && p[1] && p[2])
     {
-      VERIFY_PERMISSION (OPT_P_SETENV);
+      VERIFY_PERMISSION (OPT_P_GENERAL);
       setenv_str (es, p[1], p[2]);
     }
+  else if (streq (p[0], "setenv-safe") && p[1] && p[2])
+    {
+      VERIFY_PERMISSION (OPT_P_SETENV);
+      setenv_str_safe (es, p[1], p[2]);
+    }
   else if (streq (p[0], "mssfix"))
     {
       VERIFY_PERMISSION (OPT_P_GENERAL);