[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#23482: [PATCH 22.1] Fix buffer overflow in x-send-client-message (Bu
From: |
Lars Ingebrigtsen |
Subject: |
bug#23482: [PATCH 22.1] Fix buffer overflow in x-send-client-message (Bug#23482). |
Date: |
Tue, 11 Aug 2020 17:34:05 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Kalle Olavi Niemitalo <kon@iki.fi> writes:
> 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 <kon@iki.fi>
>
> * 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.
Sorry; it doesn't seem like you got a response to this patch at the
time.
To recap: The following will crash Emacs, so don't eval it:
(x-send-client-message nil nil nil "foo" 32 (make-list 100 0))
I can confirm that this problem is still present in Emacs 28, and that
Kalle's patch fixes it. It looks pretty straight-forward, but does
anybody have any comments here? I've included the re-spun patch for
Emacs 28 below.
> This patch is for Emacs 22.1 and includes the prominent notices
> required by clause 2a of GPLv2.
I'm not sure what that means?
> I do not intend to assign copyright to the FSF.
It's less than ten lines, so that shouldn't be necessary.
diff --git a/src/xfns.c b/src/xfns.c
index 09dcbbfb92..0203c1324f 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -5890,7 +5890,8 @@ DEFUN ("x-change-window-property",
Fx_change_window_property,
elsize = element_format == 32 ? sizeof (long) : element_format >> 3;
data = xnmalloc (nelements, elsize);
- 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 48d6215a7b..5234bccbd9 100644
--- a/src/xselect.c
+++ b/src/xselect.c
@@ -2276,23 +2276,28 @@ x_check_property_data (Lisp_Object 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 (Display *dpy, Lisp_Object data, void *ret, int format)
+x_fill_property_data (Display *dpy, Lisp_Object data, void *ret,
+ int nelements_max, int format)
{
unsigned long val;
unsigned long *d32 = (unsigned long *) ret;
unsigned short *d16 = (unsigned short *) ret;
unsigned char *d08 = (unsigned 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);
@@ -2593,7 +2598,9 @@ x_send_client_event (Lisp_Object display, Lisp_Object
dest, Lisp_Object from,
event.xclient.window = to_root ? FRAME_OUTER_WINDOW (f) : wdest;
memset (event.xclient.data.l, 0, sizeof (event.xclient.data.l));
+ /* 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 bc10043c54..db8d584781 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -1207,6 +1207,7 @@ x_mutable_colormap (Visual *visual)
extern void x_fill_property_data (Display *,
Lisp_Object,
void *,
+ int,
int);
extern Lisp_Object x_property_data_to_lisp (struct frame *,
const unsigned char *,
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
- bug#23482: [PATCH 22.1] Fix buffer overflow in x-send-client-message (Bug#23482).,
Lars Ingebrigtsen <=