Hello all,
Windows is a pain in the neck. Here's a patch that uses poll() on
all systems except Windows. This should work around the buffer overrun
vulnerability in select() described in
http://lists.nongnu.org/archive/html/chicken-users/2012-06/msg00031.html
I hope we can finally check this one off our list; it's been almost
half a year!
I've tested Spiffy on NetBSD with Slowloris, and I couldn't get it to
crash when HAVE_POSIX_POLL was defined, whereas it would crash quickly
when it wasn't (but only in a DEBUGBUILD; for some reason a normal build
won't crash so easily).
Cheers,
Peter
--
http://sjamaan.ath.cx
--
"The process of preparing programs for a digital computer
is especially attractive, not only because it can be economically
and scientifically rewarding, but also because it can be an aesthetic
experience much like composing poetry or music."
-- Donald Knuth
>From f39f851b0e52d30fe66dcb03e89800b752bf1b72 Mon Sep 17 00:00:00 2001
From: Peter Bex <peter....@xs4all.nl>
Date: Sun, 18 Nov 2012 21:03:51 +0100
Subject: [PATCH] Fix select() buffer overrun vulnerability by using POSIX
poll() on systems that support it, leaving only those few
systems vulnerable that don't (ie, only Windows).
---
Makefile.bsd | 1 +
Makefile.haiku | 1 +
Makefile.linux | 1 +
Makefile.macosx | 1 +
Makefile.solaris | 1 +
NEWS | 4 ++
scheduler.scm | 130 +++++++++++++++++++++++++++++++++++--------------------
7 files changed, 92 insertions(+), 47 deletions(-)
diff --git a/Makefile.bsd b/Makefile.bsd
index fb1f244..00a77ca 100644
--- a/Makefile.bsd
+++ b/Makefile.bsd
@@ -83,6 +83,7 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_LONG_LONG 1" >>$@
echo "#define HAVE_MEMMOVE 1" >>$@
echo "#define HAVE_MEMORY_H 1" >>$@
+ echo "#define HAVE_POSIX_POLL 1" >>$@
echo "#define HAVE_SIGACTION 1" >>$@
echo "#define HAVE_SIGSETJMP 1" >>$@
echo "#define HAVE_SIGPROCMASK 1" >>$@
diff --git a/Makefile.haiku b/Makefile.haiku
index ff06c5b..407366a 100644
--- a/Makefile.haiku
+++ b/Makefile.haiku
@@ -71,6 +71,7 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_LONG_LONG 1" >>$@
echo "#define HAVE_MEMMOVE 1" >>$@
echo "#define HAVE_MEMORY_H 1" >>$@
+ echo "#define HAVE_POSIX_POLL 1" >>$@
echo "#define HAVE_SIGACTION 1" >>$@
echo "#define HAVE_SIGSETJMP 1" >>$@
echo "#define HAVE_SIGPROCMASK 1" >>$@
diff --git a/Makefile.linux b/Makefile.linux
index b04e7f6..e944252 100644
--- a/Makefile.linux
+++ b/Makefile.linux
@@ -72,6 +72,7 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_LONG_LONG 1" >>$@
echo "#define HAVE_MEMMOVE 1" >>$@
echo "#define HAVE_MEMORY_H 1" >>$@
+ echo "#define HAVE_POSIX_POLL 1" >>$@
echo "#define HAVE_SIGACTION 1" >>$@
echo "#define HAVE_SIGSETJMP 1" >>$@
echo "#define HAVE_SIGPROCMASK 1" >>$@
diff --git a/Makefile.macosx b/Makefile.macosx
index dbdefe3..5cdf36d 100644
--- a/Makefile.macosx
+++ b/Makefile.macosx
@@ -96,6 +96,7 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_LONG_LONG 1" >>$@
echo "#define HAVE_MEMMOVE 1" >>$@
echo "#define HAVE_MEMORY_H 1" >>$@
+ echo "#define HAVE_POSIX_POLL 1" >>$@
echo "#define HAVE_SIGACTION 1" >>$@
echo "#define HAVE_SIGSETJMP 1" >>$@
echo "#define HAVE_SIGPROCMASK 1" >>$@
diff --git a/Makefile.solaris b/Makefile.solaris
index 6a92bf0..4d9f7a7 100644
--- a/Makefile.solaris
+++ b/Makefile.solaris
@@ -102,6 +102,7 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_LONG_LONG 1" >>$@
echo "#define HAVE_MEMMOVE 1" >>$@
echo "#define HAVE_MEMORY_H 1" >>$@
+ echo "#define HAVE_POSIX_POLL 1" >>$@
echo "#define HAVE_SIGACTION 1" >>$@
echo "#define HAVE_STDINT_H 1" >>$@
echo "#define HAVE_STDLIB_H 1" >>$@
diff --git a/NEWS b/NEWS
index 226d244..39c5bb2 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,9 @@
4.8.1
+- Security fixes
+ - Use POSIX poll() on systems where available. This avoids a design flaw
+ in select(); it supports no more than FD_SETSIZE descriptors.
+
- Core libraries
- Fixed EINTR handling in process-wait and when reading from file ports.
diff --git a/scheduler.scm b/scheduler.scm
index 7ff3d5f..f42ae09 100644
--- a/scheduler.scm
+++ b/scheduler.scm
@@ -31,9 +31,7 @@
(hide ready-queue-head ready-queue-tail ##sys#timeout-list
##sys#update-thread-state-buffer ##sys#restore-thread-state-buffer
remove-from-ready-queue ##sys#unblock-threads-for-i/o
##sys#force-primordial
- fdset-input-set fdset-output-set fdset-clear
- fdset-select-timeout fdset-set fdset-test
- create-fdset stderr
+ fdset-set fdset-test create-fdset stderr
##sys#clear-i/o-state-for-thread! ##sys#abandon-mutexes)
(not inline ##sys#interrupt-hook)
(unsafe)
@@ -74,9 +72,66 @@ C_word C_msleep(C_word ms) {
return C_SCHEME_TRUE;
}
#endif
+
+#ifdef HAVE_POSIX_POLL
+# include <poll.h>
+# include <assert.h>
+
+static int C_fdset_nfds;
+static struct pollfd *C_fdset_set = NULL;
+
+C_inline int C_fd_ready(int fd, int pos, int what) {
+ assert(fd == C_fdset_set[pos].fd); /* Must match position in ##sys#fd-list!
*/
+ return(C_fdset_set[pos].revents & what);
+}
+
+#define C_fd_input_ready(fd,pos) C_mk_bool(C_fd_ready(C_unfix(fd),
C_unfix(pos),POLLIN))
+#define C_fd_output_ready(fd,pos) C_mk_bool(C_fd_ready(C_unfix(fd),
C_unfix(pos),POLLOUT))
+
+C_inline int C_ready_fds_timeout(int to, double tm) {
+ return poll(C_fdset_set, C_fdset_nfds, to ? (int)tm : -1);
+}
+
+C_inline void C_prepare_fdset(int length) {
+ /* TODO: Only realloc when needed? */
+ C_fdset_set = realloc(C_fdset_set, sizeof(struct pollfd) * length);
+ if (C_fdset_set == NULL)
+ C_halt(C_SCHEME_FALSE); /* Ugly: no message */
+ C_fdset_nfds = 0;
+}
+
+/* This *must* be called in order, so position will match ##sys#fd-list */
+C_inline void C_fdset_add(int fd, int input, int output) {
+ C_fdset_set[C_fdset_nfds].events = ((input ? POLLIN : 0) | (output ? POLLOUT
: 0));
+ C_fdset_set[C_fdset_nfds++].fd = fd;
+}
+
+#else
+
+/* Shouldn't we include <sys/select.h> here? */
static fd_set C_fdset_input, C_fdset_output;
-#define C_fd_test_input(fd) C_mk_bool(FD_ISSET(C_unfix(fd), &C_fdset_input))
-#define C_fd_test_output(fd) C_mk_bool(FD_ISSET(C_unfix(fd), &C_fdset_output))
+
+#define C_fd_input_ready(fd,pos) C_mk_bool(FD_ISSET(C_unfix(fd),
&C_fdset_input))
+#define C_fd_output_ready(fd,pos) C_mk_bool(FD_ISSET(C_unfix(fd),
&C_fdset_output))
+
+C_inline int C_ready_fds_timeout(int to, double tm) {
+ struct timeval timeout;
+ timeout.tv_sec = tm / 1000;
+ timeout.tv_usec = fmod(tm, 1000) * 1000;
+ /* we use FD_SETSIZE, but really should use max fd */
+ return select(FD_SETSIZE, &C_fdset_input, &C_fdset_output, NULL, to ?
&timeout : NULL);
+}
+
+C_inline void C_prepare_fdset(int length) {
+ FD_ZERO(&C_fdset_input);
+ FD_ZERO(&C_fdset_output);
+}
+
+C_inline void C_fdset_add(int fd, int input, int output) {
+ if (input) FD_SET(fd, &C_fdset_input);
+ if (output) FD_SET(fd, &C_fdset_output);
+}
+#endif
EOF
) )
@@ -329,12 +384,12 @@ EOF
(##sys#schedule) ) )
-;;; `select()'-based blocking:
+;;; `select()/poll()'-based blocking:
(define ##sys#fd-list '()) ; ((FD1 THREAD1 ...) ...)
(define (create-fdset)
- (fdset-clear)
+ ((foreign-lambda void "C_prepare_fdset" int) (##sys#length ##sys#fd-list))
(let loop ((lst ##sys#fd-list))
(unless (null? lst)
(let ((fd (caar lst)))
@@ -345,35 +400,14 @@ EOF
(cdar lst))
(loop (cdr lst))))))
-(define fdset-select-timeout
- (foreign-lambda* int ([bool to] [double tm])
- "struct timeval timeout;"
- "timeout.tv_sec = tm / 1000;"
- "timeout.tv_usec = fmod(tm, 1000) * 1000;"
- "C_return(select(FD_SETSIZE, &C_fdset_input, &C_fdset_output, NULL, to ?
&timeout : NULL));") )
-
-(define fdset-clear
- (foreign-lambda* void ()
- "FD_ZERO(&C_fdset_input);"
- "FD_ZERO(&C_fdset_output);"))
-
-(define fdset-input-set
- (foreign-lambda* void ([int fd])
- "FD_SET(fd, &C_fdset_input);" ) )
-
-(define fdset-output-set
- (foreign-lambda* void ([int fd])
- "FD_SET(fd, &C_fdset_output);" ) )
-
(define (fdset-set fd i/o)
- (dbg "setting fdset for " fd " to " i/o)
- (case i/o
- ((#t #:input) (fdset-input-set fd))
- ((#f #:output) (fdset-output-set fd))
- ((#:all)
- (fdset-input-set fd)
- (fdset-output-set fd) )
- (else (panic "fdset-set: invalid i/o direction"))))
+ (let ((fdset-add! (foreign-lambda void "C_fdset_add" int bool bool)))
+ (dbg "setting fdset for " fd " to " i/o)
+ (case i/o
+ ((#t #:input) (fdset-add! fd #t #f))
+ ((#f #:output) (fdset-add! fd #f #t))
+ ((#:all) (fdset-add! fd #t #t))
+ (else (panic "fdset-set: invalid i/o direction")))))
(define (fdset-test inf outf i/o)
(case i/o
@@ -408,29 +442,31 @@ EOF
(fpmax 0.0 (fp- tmo1 now)) )
0.0) ) ) ; otherwise immediate timeout.
(dbg "waiting for I/O with timeout " tmo)
- (let ((n (fdset-select-timeout ; we use FD_SETSIZE, but really should use
max fd
- (or rq? to?)
- tmo)))
+ (let ((n ((foreign-lambda int "C_ready_fds_timeout" bool double)
+ (or rq? to?) tmo)))
(dbg n " fds ready")
(cond [(eq? -1 n)
- (dbg "select(2) returned with result -1" )
+ (dbg "select(2)/poll(2) returned with result -1" )
(##sys#force-primordial)]
[(fx> n 0)
(set! ##sys#fd-list
- (let loop ([n n] [lst ##sys#fd-list])
+ (let loop ((n n) (pos 0) (lst ##sys#fd-list))
(if (or (zero? n) (null? lst))
lst
- (let* ([a (car lst)]
- [fd (car a)]
- [inf (##core#inline "C_fd_test_input" fd)]
- [outf (##core#inline "C_fd_test_output" fd)])
+ (let* ((a (car lst))
+ (fd (car a))
+ ;; pos *must* match position of fd in ##sys#fd-list
+ ;; This is checked in C_fd_ready with assert()
+ (inf (##core#inline "C_fd_input_ready" fd pos))
+ (outf (##core#inline "C_fd_output_ready" fd pos)))
(dbg "fd " fd " state: input=" inf ", output=" outf)
(if (or inf outf)
(let loop2 ((threads (cdr a)) (keep '()))
(if (null? threads)
(if (null? keep)
- (loop (sub1 n) (cdr lst))
- (cons (cons fd keep) (loop (sub1 n) (cdr
lst))))
+ (loop (sub1 n) (add1 pos) (cdr lst))
+ (cons (cons fd keep)
+ (loop (sub1 n) (add1 pos) (cdr
lst))))
(let* ((t (car threads))
(p (##sys#slot t 11)) )
(dbg "checking " t " " p)
@@ -452,7 +488,7 @@ EOF
(##sys#thread-basic-unblock! t)
(loop2 (cdr threads) keep))
(else (loop2 (cdr threads) (cons t
keep)))))))
- (cons a (loop n (cdr lst))) ) ) ) ) ) ] ))) )
+ (cons a (loop n (add1 pos) (cdr lst))) ) ) ) ) ) ]
))) )
;;; Clear I/O state for unblocked thread
--
1.7.12.2
_______________________________________________
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers
