1. openvpn
  2. Commit e0b3fd49e2b5bba8cb57419a13cb75b56ac91b94
Browse code

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>

Steffan Karger authored on 2016/03/03 18:22:48
Showing 5 changed files
src/openvpn/event.c
History View file @ e0b3fd4
... ... 
@@ -873,18 +873,18 @@ se_ctl (struct event_set *es, event_t event, unsigned int rwflags, void *arg)
873 873 
       if (ses->fast)
874 874 
 	{
875 875 
 	  if (rwflags & EVENT_READ)
876 
-	    FD_SET (event, &ses->readfds);
876 
+	    openvpn_fd_set (event, &ses->readfds);
877 877 
 	  if (rwflags & EVENT_WRITE)
878 
-	    FD_SET (event, &ses->writefds);
878 
+	    openvpn_fd_set (event, &ses->writefds);
879 879 
 	}
880 880 
       else
881 881 
 	{
882 882 
 	  if (rwflags & EVENT_READ)
883 
-	    FD_SET (event, &ses->readfds);
883 
+	    openvpn_fd_set (event, &ses->readfds);
884 884 
 	  else
885 885 
 	    FD_CLR (event, &ses->readfds);
886 886 
 	  if (rwflags & EVENT_WRITE)
887 
-	    FD_SET (event, &ses->writefds);
887 
+	    openvpn_fd_set (event, &ses->writefds);
888 888 
 	  else
889 889 
 	    FD_CLR (event, &ses->writefds);
890 890 
 	}
src/openvpn/fdmisc.h
History View file @ e0b3fd4
... ... 
@@ -22,10 +22,26 @@
22 22 
  *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
23 23 
  */
24 24
25 
+#ifndef FD_MISC_H
26 
+#define FD_MISC_H
27 
+
25 28 
 #include "basic.h"
29 
+#include "error.h"
30 
+#include "syshead.h"
26 31
27 32 
 bool set_nonblock_action (int fd);
28 33 
 bool set_cloexec_action (int fd);
29 34
30 35 
 void set_nonblock (int fd);
31 36 
 void set_cloexec (int fd);
37 
+
38 
+static inline void openvpn_fd_set(int fd, fd_set *setp)
39 
+{
40 
+#ifndef WIN32 /* The Windows FD_SET() implementation does not overflow */
41 
+  ASSERT (fd >= 0 && fd < FD_SETSIZE);
42 
+#endif
43 
+  FD_SET (fd, setp);
44 
+}
45 
+#undef FD_SET /* prevent direct use of FD_SET() */
46 
+
47 
+#endif /* FD_MISC_H */
src/openvpn/proxy.c
History View file @ e0b3fd4
... ... 
@@ -92,7 +92,7 @@ recv_line (socket_descriptor_t sd,
92 92 
 	}
93 93
94 94 
       FD_ZERO (&reads);
95 
-      FD_SET (sd, &reads);
95 
+      openvpn_fd_set (sd, &reads);
96 96 
       tv.tv_sec = timeout_sec;
97 97 
       tv.tv_usec = 0;
98 98
src/openvpn/socket.c
History View file @ e0b3fd4
... ... 
@@ -1003,7 +1003,7 @@ socket_listen_accept (socket_descriptor_t sd,
1003 1003 
       struct timeval tv;
1004 1004
1005 1005 
       FD_ZERO (&reads);
1006 
-      FD_SET (sd, &reads);
1006 
+      openvpn_fd_set (sd, &reads);
1007 1007 
       tv.tv_sec = 0;
1008 1008 
       tv.tv_usec = 0;
1009 1009
... ... 
@@ -1153,7 +1153,7 @@ openvpn_connect (socket_descriptor_t sd,
1153 1153 
 	  struct timeval tv;
1154 1154
1155 1155 
 	  FD_ZERO (&writes);
1156 
-	  FD_SET (sd, &writes);
1156 
+	  openvpn_fd_set (sd, &writes);
1157 1157 
 	  tv.tv_sec = 0;
1158 1158 
 	  tv.tv_usec = 0;
1159 1159
src/openvpn/socks.c
History View file @ e0b3fd4
... ... 
@@ -134,7 +134,7 @@ socks_username_password_auth (struct socks_proxy_info *p,
134 134 
       char c;
135 135
136 136 
       FD_ZERO (&reads);
137 
-      FD_SET (sd, &reads);
137 
+      openvpn_fd_set (sd, &reads);
138 138 
       tv.tv_sec = timeout_sec;
139 139 
       tv.tv_usec = 0;
140 140
... ... 
@@ -213,7 +213,7 @@ socks_handshake (struct socks_proxy_info *p,
213 213 
       char c;
214 214
215 215 
       FD_ZERO (&reads);
216 
-      FD_SET (sd, &reads);
216 
+      openvpn_fd_set (sd, &reads);
217 217 
       tv.tv_sec = timeout_sec;
218 218 
       tv.tv_usec = 0;
219 219
... ... 
@@ -319,7 +319,7 @@ recv_socks_reply (socket_descriptor_t sd,
319 319 
       char c;
320 320
321 321 
       FD_ZERO (&reads);
322 
-      FD_SET (sd, &reads);
322 
+      openvpn_fd_set (sd, &reads);
323 323 
       tv.tv_sec = timeout_sec;
324 324 
       tv.tv_usec = 0;
325 325