curl_multi_fdset: avoid FD_SET out of bounds

If a socket is larger than FD_SETSIZE, avoid using FD_SET() on the
platforms where this is possible.

Bug: http://curl.haxx.se/bug/view.cgi?id=3413274
Reported by: Tim Starling

4 files changed, 27 insertions, 16 deletions

diff --git a/docs/libcurl/curl_multi_fdset.3 b/docs/libcurl/curl_multi_fdset.3
index fadc53812..bdb389f26 100644
--- a/docs/libcurl/curl_multi_fdset.3
+++ b/docs/libcurl/curl_multi_fdset.3
@@ -53,6 +53,14 @@ When doing select(), you should use \fBcurl_multi_timeout\fP to figure out how
 long to wait for action. Call \fIcurl_multi_perform\fP even if no activity has
 been seen on the fd_sets after the timeout expires as otherwise internal
 retries and timeouts may not work as you'd think and want.
+
+If one of the sockets used by libcurl happens to be larger than what can be
+set in an fd_set, which on POSIX systems means that the file descriptor is
+larger than FD_SETSIZE, then libcurl will try to not set it. Setting a too
+large file descriptor in an fd_set implies an out of bounds write which can
+cause crashes, or worse. The effect of NOT storing it will possibly save you
+from the crash, but will make your program NOT wait for sockets it should wait
+for...
 .SH RETURN VALUE
 CURLMcode type, general libcurl multi interface error code. See
 \fIlibcurl-errors(3)\fP
diff --git a/lib/multi.c b/lib/multi.c
index 5a1cef40d..13f5619cc 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -42,6 +42,7 @@
 #include "timeval.h"
 #include "http.h"
 #include "warnless.h"
+#include "select.h"
 
 #define _MPRINTF_REPLACE /* use our functions only */
 #include <curl/mprintf.h>
@@ -907,11 +908,11 @@ CURLMcode curl_multi_fdset(CURLM *multi_handle,
     for(i=0; i< MAX_SOCKSPEREASYHANDLE; i++) {
       curl_socket_t s = CURL_SOCKET_BAD;
 
-      if(bitmap & GETSOCK_READSOCK(i)) {
+      if((bitmap & GETSOCK_READSOCK(i)) && VALID_SOCK((sockbunch[i]))) {
         FD_SET(sockbunch[i], read_fd_set);
         s = sockbunch[i];
       }
-      if(bitmap & GETSOCK_WRITESOCK(i)) {
+      if((bitmap & GETSOCK_WRITESOCK(i)) && VALID_SOCK((sockbunch[i]))) {
         FD_SET(sockbunch[i], write_fd_set);
         s = sockbunch[i];
       }
diff --git a/lib/select.c b/lib/select.c
index 79475d539..6cecbf701 100644
--- a/lib/select.c
+++ b/lib/select.c
@@ -46,20 +46,6 @@
 #include "select.h"
 #include "warnless.h"
 
-/* Winsock and TPF sockets are not in range [0..FD_SETSIZE-1] */
-
-#if defined(USE_WINSOCK) || defined(TPF)
-#define VERIFY_SOCK(x) Curl_nop_stmt
-#else
-#define VALID_SOCK(s) (((s) >= 0) && ((s) < FD_SETSIZE))
-#define VERIFY_SOCK(x) do { \
-  if(!VALID_SOCK(x)) { \
-    SET_SOCKERRNO(EINVAL); \
-    return -1; \
-  } \
-} WHILE_FALSE
-#endif
-
 /* Convenience local macros */
 
 #define elapsed_ms  (int)curlx_tvdiff(curlx_tvnow(), initial_tv)
diff --git a/lib/select.h b/lib/select.h
index d785fb393..56729a3b2 100644
--- a/lib/select.h
+++ b/lib/select.h
@@ -96,4 +96,20 @@ int tpf_select_libcurl(int maxfds, fd_set* reads, fd_set* writes,
                        fd_set* excepts, struct timeval* tv);
 #endif
 
+/* Winsock and TPF sockets are not in range [0..FD_SETSIZE-1], which
+   unfortunately makes it impossible for us to easily check if they're valid
+*/
+#if defined(USE_WINSOCK) || defined(TPF)
+#define VALID_SOCK(x) 1
+#define VERIFY_SOCK(x) Curl_nop_stmt
+#else
+#define VALID_SOCK(s) (((s) >= 0) && ((s) < FD_SETSIZE))
+#define VERIFY_SOCK(x) do { \
+  if(!VALID_SOCK(x)) { \
+    SET_SOCKERRNO(EINVAL); \
+    return -1; \
+  } \
+} WHILE_FALSE
+#endif
+
 #endif /* __SELECT_H */