From: Steffan Karger <steffan.karger@fox-it.com>
Date: Thu, 3 Mar 2016 09:22:48 +0000 (+0100)
Subject: hardening: add safe FD_SET() wrapper openvpn_fd_set()
X-Git-Tag: v2.4_alpha1~124
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e0b3fd49e2b5bba8cb57419a13cb75b56ac91b94;p=thirdparty%2Fopenvpn.git

hardening: add safe FD_SET() wrapper openvpn_fd_set()

On many platforms (not Windows, for once), FD_SET() can write outside the
given fd_set if an fd >= FD_SETSIZE is given.  To make sure we don't do
that, add an ASSERT() to error out with a clear error message when this
does happen.

This patch was inspired by remarks about FD_SET() from Sebastian Krahmer
of the SuSE Security Team.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1456996968-29472-1-git-send-email-steffan.karger@fox-it.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11285
Signed-off-by: Gert Doering <gert@greenie.muc.de>
---

diff --git a/src/openvpn/event.c b/src/openvpn/event.c
index 34a3c451f..c6426911f 100644
--- a/src/openvpn/event.c
+++ b/src/openvpn/event.c
@@ -873,18 +873,18 @@ se_ctl (struct event_set *es, event_t event, unsigned int rwflags, void *arg)
       if (ses->fast)
 	{
 	  if (rwflags & EVENT_READ)
-	    FD_SET (event, &ses->readfds);
+	    openvpn_fd_set (event, &ses->readfds);
 	  if (rwflags & EVENT_WRITE)
-	    FD_SET (event, &ses->writefds);
+	    openvpn_fd_set (event, &ses->writefds);
 	}
       else
 	{
 	  if (rwflags & EVENT_READ)
-	    FD_SET (event, &ses->readfds);
+	    openvpn_fd_set (event, &ses->readfds);
 	  else
 	    FD_CLR (event, &ses->readfds);
 	  if (rwflags & EVENT_WRITE)
-	    FD_SET (event, &ses->writefds);
+	    openvpn_fd_set (event, &ses->writefds);
 	  else
 	    FD_CLR (event, &ses->writefds);
 	}
diff --git a/src/openvpn/fdmisc.h b/src/openvpn/fdmisc.h
index 4b6b6d04f..13d6552d2 100644
--- a/src/openvpn/fdmisc.h
+++ b/src/openvpn/fdmisc.h
@@ -22,10 +22,26 @@
  *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#ifndef FD_MISC_H
+#define FD_MISC_H
+
 #include "basic.h"
+#include "error.h"
+#include "syshead.h"
 
 bool set_nonblock_action (int fd);
 bool set_cloexec_action (int fd);
 
 void set_nonblock (int fd);
 void set_cloexec (int fd);
+
+static inline void openvpn_fd_set(int fd, fd_set *setp)
+{
+#ifndef WIN32 /* The Windows FD_SET() implementation does not overflow */
+  ASSERT (fd >= 0 && fd < FD_SETSIZE);
+#endif
+  FD_SET (fd, setp);
+}
+#undef FD_SET /* prevent direct use of FD_SET() */
+
+#endif /* FD_MISC_H */
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index 2568e1914..8ff645809 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -92,7 +92,7 @@ recv_line (socket_descriptor_t sd,
 	}
 
       FD_ZERO (&reads);
-      FD_SET (sd, &reads);
+      openvpn_fd_set (sd, &reads);
       tv.tv_sec = timeout_sec;
       tv.tv_usec = 0;
 
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 714a847a9..9bcf4d439 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1003,7 +1003,7 @@ socket_listen_accept (socket_descriptor_t sd,
       struct timeval tv;
 
       FD_ZERO (&reads);
-      FD_SET (sd, &reads);
+      openvpn_fd_set (sd, &reads);
       tv.tv_sec = 0;
       tv.tv_usec = 0;
 
@@ -1153,7 +1153,7 @@ openvpn_connect (socket_descriptor_t sd,
 	  struct timeval tv;
 
 	  FD_ZERO (&writes);
-	  FD_SET (sd, &writes);
+	  openvpn_fd_set (sd, &writes);
 	  tv.tv_sec = 0;
 	  tv.tv_usec = 0;
 
diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
index cef7a35e7..a9d04aef3 100644
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -134,7 +134,7 @@ socks_username_password_auth (struct socks_proxy_info *p,
       char c;
 
       FD_ZERO (&reads);
-      FD_SET (sd, &reads);
+      openvpn_fd_set (sd, &reads);
       tv.tv_sec = timeout_sec;
       tv.tv_usec = 0;
 
@@ -213,7 +213,7 @@ socks_handshake (struct socks_proxy_info *p,
       char c;
 
       FD_ZERO (&reads);
-      FD_SET (sd, &reads);
+      openvpn_fd_set (sd, &reads);
       tv.tv_sec = timeout_sec;
       tv.tv_usec = 0;
 
@@ -319,7 +319,7 @@ recv_socks_reply (socket_descriptor_t sd,
       char c;
 
       FD_ZERO (&reads);
-      FD_SET (sd, &reads);
+      openvpn_fd_set (sd, &reads);
       tv.tv_sec = timeout_sec;
       tv.tv_usec = 0;