GNU bug report logs - #8335
buffer overrun in (x-change-window-property "FOO" '(0 bad))

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8335 in the body.
You can then email your comments to 8335 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: buffer overrun in (x-change-window-property "FOO" '(0 bad))
Date: Wed, 23 Mar 2011 18:13:57 -0700

src/xselect.c's function x_check_property_data has a coding error, in
that it never reports an error.  This can lead to corrupted memory.
For example, the Lisp code (x-change-window-property "FOO" '(0 bad))
internally does an malloc (0) and then stores through the resulting
pointer.

This bug was found by static analysis, using gcc -Wstrict-overflow
(GCC 4.5.2, x86-64).

I plan to fix it with the following patch.

* xselect.c (x_check_property_data): Don't return wrong size.
=== modified file 'src/xselect.c'
--- src/xselect.c	2011-03-10 01:36:58 +0000
+++ src/xselect.c	2011-03-24 01:04:41 +0000
@@ -2190,7 +2190,8 @@
 ***********************************************************************/
 /* Check that lisp values are of correct type for x_fill_property_data.
    That is, number, string or a cons with two numbers (low and high 16
-   bit parts of a 32 bit number).  */
+   bit parts of a 32 bit number).  Return the number of items in DATA,
+   or -1 if there is an error.  */

 int
 x_check_property_data (Lisp_Object data)
@@ -2198,15 +2199,16 @@
   Lisp_Object iter;
   int size = 0;

-  for (iter = data; CONSP (iter) && size != -1; iter = XCDR (iter), ++size)
+  for (iter = data; CONSP (iter); iter = XCDR (iter))
     {
       Lisp_Object o = XCAR (iter);

       if (! NUMBERP (o) && ! STRINGP (o) && ! CONSP (o))
-        size = -1;
+        return -1;
       else if (CONSP (o) &&
                (! NUMBERP (XCAR (o)) || ! NUMBERP (XCDR (o))))
-        size = -1;
+        return -1;
+      size++;
     }

   return size;

Message #10 received at 8335-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 8344-done <at> debbugs.gnu.org, 8336-done <at> debbugs.gnu.org, 
	8335-done <at> debbugs.gnu.org
Subject: fix merged to trunk
Date: Tue, 29 Mar 2011 17:53:19 -0700

I committed a fix to the trunk for this,
as part of a recent merge (bzr 103776).

For Bug#8344, the merge uses size_t rather
than EMACS_INT for argument counts as I proposed earlier,
since the argument counts are always nonnegative
and are limited just by sizes that can be counted
at the C level.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.