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

[Paul Eggert]

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;