From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Lars Ingebrigtsen Newsgroups: gmane.emacs.bugs 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 Message-ID: <87y2mlryc2.fsf@gnus.org> References: <87r3dcenux.fsf@Niukka.kon.iki.fi> <87a8jyeadw.fsf@Niukka.kon.iki.fi> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1622"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 23482@debbugs.gnu.org To: Kalle Olavi Niemitalo Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Aug 11 17:35:19 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1k5WJ5-0000FA-Q3 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 11 Aug 2020 17:35:19 +0200 Original-Received: from localhost ([::1]:35772 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1k5WJ4-0004SD-P8 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 11 Aug 2020 11:35:18 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58078) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1k5WIo-0004QT-1j for bug-gnu-emacs@gnu.org; Tue, 11 Aug 2020 11:35:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:58169) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1k5WIn-00032D-Nz for bug-gnu-emacs@gnu.org; Tue, 11 Aug 2020 11:35:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1k5WIn-0003W4-IS for bug-gnu-emacs@gnu.org; Tue, 11 Aug 2020 11:35:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Lars Ingebrigtsen Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 11 Aug 2020 15:35:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23482 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 23482-submit@debbugs.gnu.org id=B23482.159716006213448 (code B ref 23482); Tue, 11 Aug 2020 15:35:01 +0000 Original-Received: (at 23482) by debbugs.gnu.org; 11 Aug 2020 15:34:22 +0000 Original-Received: from localhost ([127.0.0.1]:41479 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1k5WI9-0003Up-Qg for submit@debbugs.gnu.org; Tue, 11 Aug 2020 11:34:22 -0400 Original-Received: from quimby.gnus.org ([95.216.78.240]:33828) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1k5WI8-0003Uc-Df for 23482@debbugs.gnu.org; Tue, 11 Aug 2020 11:34:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnus.org; s=20200322; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=lgL0Ictt7uLB5wlKxrRckfymyfUD+ZQeKTbI0dM5TTk=; b=jW+1sIK1id8QlEefD4RRlJ8Nkk jn3Krex+X2IE3fXP+d9I21jbQ2gorT5whbYtpnvRSivXi4aV0ABEZ4cEKa7k5YHpOeZvKWk1WkHoG F1KCkJBupTV2mOL5MLVG24s2HUQ+eq5FIzpyjEF4vnpZ60sIB/DDYhTD3sOfQEMIW0Wo=; Original-Received: from cm-84.212.202.86.getinternet.no ([84.212.202.86] helo=xo) by quimby with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1k5WHz-0005fV-EB; Tue, 11 Aug 2020 17:34:14 +0200 In-Reply-To: <87a8jyeadw.fsf@Niukka.kon.iki.fi> (Kalle Olavi Niemitalo's message of "Tue, 10 May 2016 08:43:07 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:184671 Archived-At: Kalle Olavi Niemitalo 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 > > * 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