Subject: [PATCH 22.1] Fix buffer overflow in x-send-client-message (Bug#23482).

From: Kalle Olavi Niemitalo <[email-masked]>
Date: Tue, 10 May 2016 08:43:07 +0300

The docstring already said that excessive values are ignored,
but they instead overflowed the buffer.

This does not seem a security vulnerability though, because Emacs fully
trusts Emacs Lisp code, and if some Emacs Lisp code sends client
messages based on untrusted data, then that's already a bug of its own.

2016-05-08  Kalle Olavi Niemitalo  <[email-masked]>

	* xselect.c (x_fill_property_data): Add parameter NELEMENTS_MAX.
	* xterm.h (x_fill_property_data): Update prototype.
	* xselect.c (Fx_send_client_event): Update call.  This fixes
	  a buffer overflow in event.xclient.data.
	* xfns.c (Fx_change_window_property): Update call.
---
This patch is for Emacs 22.1 and includes the prominent notices
required by clause 2a of GPLv2. 
I do not intend to assign copyright to the FSF.

In Emacs 22.1, Fx_send_client_event has other bugs that this
patch does not fix.  It should clear event.xclient.data.l rather
than event.xclient.data.b, and the mask 0xffff in events sent to
the root window does not include the SubstructureNotify and
SubstructureRedirect bits required by "Extended Window Manager
Hints" version 1.1.

Date: Sun, 8 May 2016 11:33:44 +0300

 src/xfns.c    |  5 ++++-
 src/xselect.c | 17 +++++++++++++----
 src/xterm.h   |  3 +++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/xfns.c b/src/xfns.c
index d269dfb..00e28db 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -19,6 +19,8 @@ along with GNU Emacs; see the file COPYING.  If not, write to
 the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
 Boston, MA 02110-1301, USA.  */
 
+/* Modified on 2016-05-08 by Kalle Olavi Niemitalo.  */
+
 #include <config.h>
 #include <stdio.h>
 #include <math.h>
@@ -4255,7 +4257,8 @@ Value is VALUE.  */)
            converts to 32 bits before sending to the X server.  */
         data = (unsigned char *) xmalloc (nelements * sizeof(long));
 
-      x_fill_property_data (FRAME_X_DISPLAY (f), value, data, element_format);
+      x_fill_property_data (FRAME_X_DISPLAY (f), value, data, nelements,
+                            element_format);
     }
   else
     {
diff --git a/src/xselect.c b/src/xselect.c
index 3fe109a..5d4ef9c 100644
--- a/src/xselect.c
+++ b/src/xselect.c
@@ -21,6 +21,7 @@ Boston, MA 02110-1301, USA.  */

 /* Rewritten by jwz */
+/* Modified on 2016-05-08 by Kalle Olavi Niemitalo.  */
 
 #include <config.h>
 #include <stdio.h>      /* termhooks.h needs this */
@@ -2526,27 +2527,32 @@ x_check_property_data (data)
 
    DPY is the display use to look up X atoms.
    DATA is a Lisp list of values to be converted.
-   RET is the C array that contains the converted values.  It is assumed
-   it is big enough to hold all values.
+   RET is the C array that contains the converted values.
+   NELEMENTS_MAX is the number of values that will fit in RET.
+   Any excess values in DATA are ignored.
    FORMAT is 8, 16 or 32 and denotes char/short/long for each C value to
    be stored in RET.  Note that long is used for 32 even if long is more
    than 32 bits (see man pages for XChangeProperty, XGetWindowProperty and
    XClientMessageEvent).  */
 
 void
-x_fill_property_data (dpy, data, ret, format)
+x_fill_property_data (dpy, data, ret, nelements_max, format)
      Display *dpy;
      Lisp_Object data;
      void *ret;
+     int nelements_max;
      int format;
 {
   long val;
   long  *d32 = (long  *) ret;
   short *d16 = (short *) ret;
   char  *d08 = (char  *) ret;
+  int nelements;
   Lisp_Object iter;
 
-  for (iter = data; CONSP (iter); iter = XCDR (iter))
+  for (iter = data, nelements = 0;
+       CONSP (iter) && nelements < nelements_max;
+       iter = XCDR (iter), nelements++)
     {
       Lisp_Object o = XCAR (iter);
 
@@ -2883,7 +2889,10 @@ are ignored.  */)

   memset (event.xclient.data.b, 0, sizeof (event.xclient.data.b));
+
+  /* event.xclient.data can hold 20 chars, 10 shorts, or 5 longs.  */
   x_fill_property_data (dpyinfo->display, values, event.xclient.data.b,
+                        5 * 32 / event.xclient.format,
                         event.xclient.format);
 
   /* If event mask is 0 the event is sent to the client that created
diff --git a/src/xterm.h b/src/xterm.h
index 13b0b49..968ead7 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -19,6 +19,8 @@ along with GNU Emacs; see the file COPYING.  If not, write to
 the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
 Boston, MA 02110-1301, USA.  */
 
+/* Modified on 2016-05-08 by Kalle Olavi Niemitalo.  */
+
 #include <X11/Xlib.h>
 #include <X11/cursorfont.h>
 
@@ -1032,6 +1034,7 @@ extern int x_check_property_data P_ ((Lisp_Object));
 extern void x_fill_property_data P_ ((Display *,
                                       Lisp_Object,
                                       void *,
+                                      int,
                                       int));
 extern Lisp_Object x_property_data_to_lisp P_ ((struct frame *,
                                                 unsigned char *,
-- 
2.1.4