Bug#733015: [libsdl2-2.0-0] SDL2 X11 driver buffer overflow with large X11 file descriptor

[Sven Eckelmann]

Package: libsdl2-2.0-0
Version: 2.0.0+dfsg1-3
Severity: normal
Tags: patch

I have occasional crashes here caused by the X11 backend of SDL2. It seems to 
be caused by the X11_Pending function trying to add a high number (> 1024) 
file descriptor to a fd_set before doing a select on it to avoid busy waiting 
on X11 events. This causes a buffer overflow because the file descriptor is 
larger (or equal) than the limit FD_SETSIZE.

Attached is a possible workaround patch.

Please also keep in mind that fd_set are also used in following files which 
may have similar problems.

src/audio/bsd/SDL_bsdaudio.c
src/audio/paudio/SDL_paudio.c
src/audio/qsa/SDL_qsa_audio.c
src/audio/sun/SDL_sunaudio.c
src/joystick/linux/SDL_sysjoystick.c


--- System information. ---
Architecture: amd64
Kernel:       Linux 3.11-2-amd64

Debian Release: jessie/sid
  500 unstable        http.debian.net 
    1 unstable        www.deb-multimedia.org 

--- Package information. ---
Depends                  (Version) | Installed
==================================-+-==================
libasound2             (>= 1.0.16) | 
libc6                    (>= 2.15) | 
libpulse0              (>= 0.99.1) | 
libts-0.0-0               (>= 1.0) | 
libx11-6         (>= 2:1.2.99.901) | 
libxcursor1             (>> 1.1.2) | 
libxext6                           | 
libxi6             (>= 2:1.2.99.4) | 
libxinerama1                       | 
libxrandr2            (>= 2:1.2.0) | 
libxss1                            | 
libxxf86vm1                        | 


Package's Recommends field is empty.

Package's Suggests field is empty.
Description: Don't add descriptor over FD_SETSIZE to fd_set
 ConnectionNumber in X11_Pending may return a value outside of the range [0,
 FD_SETSIZE). This value cannot be stored inside a fd_set and will crash the
 program.
 .
 This buffer overflow problem occasionally happens when a lot of file
 descriptors are used.
Author: Sven Eckelmann <s...@narfation.org>

---
diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c
index 818ab2e21d96fa80c0b6ba72551198e5e9f925b2..5a983208a1a7c5d7d3a1a88bfac0c4337a2f4ed1 100644
--- a/src/video/x11/SDL_x11events.c
+++ b/src/video/x11/SDL_x11events.c
@@ -917,6 +917,9 @@ X11_Pending(Display * display)
         fd_set fdset;
 
         x11_fd = ConnectionNumber(display);
+        if (x11_fd >= FD_SETSIZE || x11_fd < 0)
+            return 0;
+
         FD_ZERO(&fdset);
         FD_SET(x11_fd, &fdset);
         if (select(x11_fd + 1, &fdset, NULL, NULL, &zero_time) == 1) {

signature.asc
Description: This is a digitally signed message part.