From: Steffan Karger <steffan.kar...@fox-it.com>
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.kar...@fox-it.com>
---
src/openvpn/event.c | 8 ++++----
src/openvpn/fdmisc.h | 16 ++++++++++++++++
src/openvpn/proxy.c | 2 +-
src/openvpn/socket.c | 4 ++--
src/openvpn/socks.c | 6 +++---
5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/openvpn/event.c b/src/openvpn/event.c
index 34a3c45..c642691 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 4b6b6d0..13d6552 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 2568e19..8ff6458 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 714a847..9bcf4d4 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 cef7a35..a9d04ae 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;
--
2.5.0
