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>
(cherry picked from commit e0b3fd49e2b5bba8cb57419a13cb75b56ac91b94)

diff --git a/src/openvpn/event.c b/src/openvpn/event.c
index 34a3c451ff7915f39d6296556b7717761a45b7e0..c6426911f2401836872b01a0809983d054cb2373 100644 (file)
--- 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 4b6b6d04f7a6c11f5721fd393477d28763aa71e2..13d6552d29c9ce1ce84e81c8899ebfe61e6c767e 100644 (file)
--- 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 95d71531bc2ffbbfd096da669e8ff931b6d333eb..89989d138e62dd373bc0584cd9da0dee36033866 100644 (file)
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -94,7 +94,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 6b64a0f713b80e229dd049e08ed13b668a74bdcd..d110e90f20ef259c9fa0d5b676b2d80320bf54e5 100644 (file)
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -842,7 +842,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;
 
@@ -938,7 +938,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 117ee1d50a9ff739a1d42768836093053ea0b1d4..57dc02acfa110219220c730ec5f019c3fa438035 100644 (file)
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -136,7 +136,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;
 
@@ -215,7 +215,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;
 
@@ -321,7 +321,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;