* xselect.c: Integer and memory overflow issues.

(X_LONG_SIZE, X_USHRT_MAX, X_ULONG_MAX): New macros.
Use them to make the following changes clearer.
(MAX_SELECTION_QUANTUM): Make the other bounds on this value clearer.
This change doesn't affect the value now, but it may help remind
future maintainers not to raise the value too much later.
(SELECTION_QUANTUM): Remove, replacing with ...
(selection_quantum): ... new function, which avoids overflow.
All uses changed.
(struct selection_data.size): Now ptrdiff_t, not int, to avoid
assumption that selection length fits in 'int'.
(x_reply_selection_request, x_handle_selection_request)
(x_get_window_property, receive_incremental_selection)
(x_get_window_property_as_lisp_data, selection_data_to_lisp_data)
(lisp_data_to_selection_data, clean_local_selection_data):
Use ptrdiff_t, not int, to record length of selection.
(x_reply_selection_request, x_get_window_property)
(receive_incremental_selection, x_property_data_to_lisp):
Redo calculations to avoid overflow.
(x_reply_selection_request): When sending hint, ceiling it at
X_ULONG_MAX rather than relying on wraparound overflow to send
something.
(x_get_window_property, receive_incremental_selection)
(lisp_data_to_selection_data, x_property_data_to_lisp):
Check for size-calculation overflow.
(x_get_window_property, receive_incremental_selection)
(lisp_data_to_selection_data, Fx_register_dnd_atom):
Don't store size until memory allocation succeeds.
(x_get_window_property): Plug memory leak on memory exhaustion.
Don't double-block input; malloc is safe here.  Don't assume 2**34
- 4 fits in unsigned long.  Add an xassert to check
XGetWindowProperty overflow.  Be more careful about overflow
calculations, and distinguish size from memory overflow better.
(receive_incremental_selection): When tracing, don't assume
unsigned int is less than INT_MAX.
(x_selection_data_to_lisp_data): Remove unnecessary (and in theory
harmful) conversions of unsigned short to int.
(lisp_data_to_selection_data): Don't assume that integers
in the range -65535 through -1 fit in an X unsigned short.
Don't assume that ULONG_MAX == X_ULONG_MAX.  Don't store into
result parameters unless successful.  Rely on cons_to_unsigned
to report problems with elements; the old code wasn't right anyway.
(x_check_property_data): Check for int overflow; we cannot use
a wider type due to X limits.
(x_handle_dnd_message): Use unsigned int, to avoid int overflow.

2 files changed, 232 insertions, 121 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 09ee5a8e4dc..f8e5ca78b33 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,51 @@
 2011-07-29  Paul Eggert  <eggert@cs.ucla.edu>
 
+        * xselect.c: Integer and memory overflow issues.
+        (X_LONG_SIZE, X_USHRT_MAX, X_ULONG_MAX): New macros.
+        Use them to make the following changes clearer.
+        (MAX_SELECTION_QUANTUM): Make the other bounds on this value clearer.
+        This change doesn't affect the value now, but it may help remind
+        future maintainers not to raise the value too much later.
+        (SELECTION_QUANTUM): Remove, replacing with ...
+        (selection_quantum): ... new function, which avoids overflow.
+        All uses changed.
+        (struct selection_data.size): Now ptrdiff_t, not int, to avoid
+        assumption that selection length fits in 'int'.
+        (x_reply_selection_request, x_handle_selection_request)
+        (x_get_window_property, receive_incremental_selection)
+        (x_get_window_property_as_lisp_data, selection_data_to_lisp_data)
+        (lisp_data_to_selection_data, clean_local_selection_data):
+        Use ptrdiff_t, not int, to record length of selection.
+        (x_reply_selection_request, x_get_window_property)
+        (receive_incremental_selection, x_property_data_to_lisp):
+        Redo calculations to avoid overflow.
+        (x_reply_selection_request): When sending hint, ceiling it at
+        X_ULONG_MAX rather than relying on wraparound overflow to send
+        something.
+        (x_get_window_property, receive_incremental_selection)
+        (lisp_data_to_selection_data, x_property_data_to_lisp):
+        Check for size-calculation overflow.
+        (x_get_window_property, receive_incremental_selection)
+        (lisp_data_to_selection_data, Fx_register_dnd_atom):
+        Don't store size until memory allocation succeeds.
+        (x_get_window_property): Plug memory leak on memory exhaustion.
+        Don't double-block input; malloc is safe here.  Don't assume 2**34
+        - 4 fits in unsigned long.  Add an xassert to check
+        XGetWindowProperty overflow.  Be more careful about overflow
+        calculations, and distinguish size from memory overflow better.
+        (receive_incremental_selection): When tracing, don't assume
+        unsigned int is less than INT_MAX.
+        (x_selection_data_to_lisp_data): Remove unnecessary (and in theory
+        harmful) conversions of unsigned short to int.
+        (lisp_data_to_selection_data): Don't assume that integers
+        in the range -65535 through -1 fit in an X unsigned short.
+        Don't assume that ULONG_MAX == X_ULONG_MAX.  Don't store into
+        result parameters unless successful.  Rely on cons_to_unsigned
+        to report problems with elements; the old code wasn't right anyway.
+        (x_check_property_data): Check for int overflow; we cannot use
+        a wider type due to X limits.
+        (x_handle_dnd_message): Use unsigned int, to avoid int overflow.
+
         * xrdb.c: Integer and memory overflow issues.
         (magic_file_p): Plug memory leak on size overflow.
         (get_environ_db): Don't assume path length fits in int,
diff --git a/src/xselect.c b/src/xselect.c
index f63977a73de..d8b7b077a8c 100644
--- a/src/xselect.c
+++ b/src/xselect.c
@@ -66,22 +66,15 @@ static Lisp_Object wait_for_property_change_unwind (Lisp_Object);
 static void wait_for_property_change (struct prop_location *);
 static Lisp_Object x_get_foreign_selection (Lisp_Object, Lisp_Object,
                                             Lisp_Object, Lisp_Object);
-static void x_get_window_property (Display *, Window, Atom,
-                                   unsigned char **, int *,
-                                   Atom *, int *, unsigned long *, int);
-static void receive_incremental_selection (Display *, Window, Atom,
-                                           Lisp_Object, unsigned,
-                                           unsigned char **, int *,
-                                           Atom *, int *, unsigned long *);
 static Lisp_Object x_get_window_property_as_lisp_data (Display *,
                                                        Window, Atom,
                                                        Lisp_Object, Atom);
 static Lisp_Object selection_data_to_lisp_data (Display *,
                                                 const unsigned char *,
-                                                int, Atom, int);
+                                                ptrdiff_t, Atom, int);
 static void lisp_data_to_selection_data (Display *, Lisp_Object,
                                          unsigned char **, Atom *,
-                                         unsigned *, int *, int *);
+                                         ptrdiff_t *, int *, int *);
 static Lisp_Object clean_local_selection_data (Lisp_Object);
 
 /* Printing traces to stderr.  */
@@ -114,15 +107,37 @@ static Lisp_Object Qcompound_text_with_extensions;
 static Lisp_Object Qforeign_selection;
 static Lisp_Object Qx_lost_selection_functions, Qx_sent_selection_functions;
 
+/* Bytes needed to represent 'long' data.  This is as per libX11; it
+   is not necessarily sizeof (long).  */
+#define X_LONG_SIZE 4
+
+/* Maximum unsigned 'short' and 'long' values suitable for libX11.  */
+#define X_USHRT_MAX 0xffff
+#define X_ULONG_MAX 0xffffffff
+
 /* If this is a smaller number than the max-request-size of the display,
    emacs will use INCR selection transfer when the selection is larger
    than this.  The max-request-size is usually around 64k, so if you want
    emacs to use incremental selection transfers when the selection is
    smaller than that, set this.  I added this mostly for debugging the
-   incremental transfer stuff, but it might improve server performance.  */
-#define MAX_SELECTION_QUANTUM 0xFFFFFF
+   incremental transfer stuff, but it might improve server performance.
+
+   This value cannot exceed INT_MAX / max (X_LONG_SIZE, sizeof (long))
+   because it is multiplied by X_LONG_SIZE and by sizeof (long) in
+   subscript calculations.  Similarly for PTRDIFF_MAX - 1 or SIZE_MAX
+   - 1 in place of INT_MAX.  */
+#define MAX_SELECTION_QUANTUM                                           \
+  ((int) min (0xFFFFFF, (min (INT_MAX, min (PTRDIFF_MAX, SIZE_MAX) - 1) \
+                         / max (X_LONG_SIZE, sizeof (long)))))
 
-#define SELECTION_QUANTUM(dpy) ((XMaxRequestSize(dpy) << 2) - 100)
+static int
+selection_quantum (Display *display)
+{
+  long mrs = XMaxRequestSize (display);
+  return (mrs < MAX_SELECTION_QUANTUM / X_LONG_SIZE + 25
+          ? (mrs - 25) * X_LONG_SIZE
+          : MAX_SELECTION_QUANTUM);
+}
 
 #define LOCAL_SELECTION(selection_symbol,dpyinfo)                       \
   assq_no_quit (selection_symbol, dpyinfo->terminal->Vselection_alist)
@@ -477,7 +492,7 @@ static struct x_display_info *selection_request_dpyinfo;
 struct selection_data
 {
   unsigned char *data;
-  unsigned int size;
+  ptrdiff_t size;
   int format;
   Atom type;
   int nofree;
@@ -581,14 +596,11 @@ x_reply_selection_request (struct input_event *event, struct x_display_info *dpy
   XSelectionEvent *reply = &(reply_base.xselection);
   Display *display = SELECTION_EVENT_DISPLAY (event);
   Window window = SELECTION_EVENT_REQUESTOR (event);
-  int bytes_remaining;
-  int max_bytes = SELECTION_QUANTUM (display);
+  ptrdiff_t bytes_remaining;
+  int max_bytes = selection_quantum (display);
   int count = SPECPDL_INDEX ();
   struct selection_data *cs;
 
-  if (max_bytes > MAX_SELECTION_QUANTUM)
-    max_bytes = MAX_SELECTION_QUANTUM;
-
   reply->type = SelectionNotify;
   reply->display = display;
   reply->requestor = window;
@@ -616,11 +628,12 @@ x_reply_selection_request (struct input_event *event, struct x_display_info *dpy
       if (cs->property == None)
         continue;
 
-      bytes_remaining = cs->size * (cs->format / 8);
+      bytes_remaining = cs->size;
+      bytes_remaining *= cs->format >> 3;
       if (bytes_remaining <= max_bytes)
         {
           /* Send all the data at once, with minimal handshaking.  */
-          TRACE1 ("Sending all %d bytes", bytes_remaining);
+          TRACE1 ("Sending all %"pD"d bytes", bytes_remaining);
           XChangeProperty (display, window, cs->property,
                            cs->type, cs->format, PropModeReplace,
                            cs->data, cs->size);
@@ -628,9 +641,9 @@ x_reply_selection_request (struct input_event *event, struct x_display_info *dpy
       else
         {
           /* Send an INCR tag to initiate incremental transfer.  */
-          long value[1];
+          unsigned long value[1];
 
-          TRACE2 ("Start sending %d bytes incrementally (%s)",
+          TRACE2 ("Start sending %"pD"d bytes incrementally (%s)",
                   bytes_remaining, XGetAtomName (display, cs->property));
           cs->wait_object
             = expect_property_change (display, window, cs->property,
@@ -638,7 +651,7 @@ x_reply_selection_request (struct input_event *event, struct x_display_info *dpy
 
           /* XChangeProperty expects an array of long even if long is
              more than 32 bits.  */
-          value[0] = bytes_remaining;
+          value[0] = min (bytes_remaining, X_ULONG_MAX);
           XChangeProperty (display, window, cs->property,
                            dpyinfo->Xatom_INCR, 32, PropModeReplace,
                            (unsigned char *) value, 1);
@@ -672,7 +685,8 @@ x_reply_selection_request (struct input_event *event, struct x_display_info *dpy
         int had_errors = x_had_errors_p (display);
         UNBLOCK_INPUT;
 
-        bytes_remaining = cs->size * format_bytes;
+        bytes_remaining = cs->size;
+        bytes_remaining *= format_bytes;
 
         /* Wait for the requester to ack by deleting the property.
            This can run Lisp code (process handlers) or signal.  */
@@ -810,7 +824,7 @@ x_handle_selection_request (struct input_event *event)
          non-None property.  */
       Window requestor = SELECTION_EVENT_REQUESTOR (event);
       Lisp_Object multprop;
-      int j, nselections;
+      ptrdiff_t j, nselections;
 
       if (property == None) goto DONE;
       multprop
@@ -1269,19 +1283,28 @@ x_get_foreign_selection (Lisp_Object selection_symbol, Lisp_Object target_type,
 
 static void
 x_get_window_property (Display *display, Window window, Atom property,
-                       unsigned char **data_ret, int *bytes_ret,
+                       unsigned char **data_ret, ptrdiff_t *bytes_ret,
                        Atom *actual_type_ret, int *actual_format_ret,
                        unsigned long *actual_size_ret, int delete_p)
 {
-  int total_size;
+  ptrdiff_t total_size;
   unsigned long bytes_remaining;
-  int offset = 0;
+  ptrdiff_t offset = 0;
+  unsigned char *data = 0;
   unsigned char *tmp_data = 0;
   int result;
-  int buffer_size = SELECTION_QUANTUM (display);
+  int buffer_size = selection_quantum (display);
+
+  /* Wide enough to avoid overflow in expressions using it.  */
+  ptrdiff_t x_long_size = X_LONG_SIZE;
 
-  if (buffer_size > MAX_SELECTION_QUANTUM)
-    buffer_size = MAX_SELECTION_QUANTUM;
+  /* Maximum value for TOTAL_SIZE.  It cannot exceed PTRDIFF_MAX - 1
+     and SIZE_MAX - 1, for an extra byte at the end.  And it cannot
+     exceed LONG_MAX * X_LONG_SIZE, for XGetWindowProperty.  */
+  ptrdiff_t total_size_max =
+    ((min (PTRDIFF_MAX, SIZE_MAX) - 1) / x_long_size < LONG_MAX
+     ? min (PTRDIFF_MAX, SIZE_MAX) - 1
+     : LONG_MAX * x_long_size);
 
   BLOCK_INPUT;
 
@@ -1292,49 +1315,44 @@ x_get_window_property (Display *display, Window window, Atom property,
                                actual_size_ret,
                                &bytes_remaining, &tmp_data);
   if (result != Success)
-    {
-      UNBLOCK_INPUT;
-      *data_ret = 0;
-      *bytes_ret = 0;
-      return;
-    }
+    goto done;
 
   /* This was allocated by Xlib, so use XFree.  */
   XFree ((char *) tmp_data);
 
   if (*actual_type_ret == None || *actual_format_ret == 0)
-    {
-      UNBLOCK_INPUT;
-      return;
-    }
+    goto done;
 
-  total_size = bytes_remaining + 1;
-  *data_ret = (unsigned char *) xmalloc (total_size);
+  if (total_size_max < bytes_remaining)
+    goto size_overflow;
+  total_size = bytes_remaining;
+  data = malloc (total_size + 1);
+  if (! data)
+    goto memory_exhausted;
 
   /* Now read, until we've gotten it all.  */
   while (bytes_remaining)
     {
-#ifdef TRACE_SELECTION
-      unsigned long last = bytes_remaining;
-#endif
+      ptrdiff_t bytes_gotten;
+      int bytes_per_item;
       result
         = XGetWindowProperty (display, window, property,
-                              (long)offset/4, (long)buffer_size/4,
+                              offset / X_LONG_SIZE,
+                              buffer_size / X_LONG_SIZE,
                               False,
                               AnyPropertyType,
                               actual_type_ret, actual_format_ret,
                               actual_size_ret, &bytes_remaining, &tmp_data);
 
-      TRACE2 ("Read %lu bytes from property %s",
-              last - bytes_remaining,
-              XGetAtomName (display, property));
-
       /* If this doesn't return Success at this point, it means that
          some clod deleted the selection while we were in the midst of
          reading it.  Deal with that, I guess.... */
       if (result != Success)
         break;
 
+      bytes_per_item = *actual_format_ret >> 3;
+      xassert (*actual_size_ret <= buffer_size / bytes_per_item);
+
       /* The man page for XGetWindowProperty says:
          "If the returned format is 32, the returned data is represented
           as a long array and should be cast to that type to obtain the
@@ -1348,32 +1366,61 @@ x_get_window_property (Display *display, Window window, Atom property,
          The bytes and offsets passed to XGetWindowProperty refers to the
          property and those are indeed in 32 bit quantities if format is 32.  */
 
+      bytes_gotten = *actual_size_ret;
+      bytes_gotten *= bytes_per_item;
+
+      TRACE2 ("Read %"pD"d bytes from property %s",
+              bytes_gotten, XGetAtomName (display, property));
+
+      if (total_size - offset < bytes_gotten)
+        {
+          unsigned char *data1;
+          ptrdiff_t remaining_lim = total_size_max - offset - bytes_gotten;
+          if (remaining_lim < 0 || remaining_lim < bytes_remaining)
+            goto size_overflow;
+          total_size = offset + bytes_gotten + bytes_remaining;
+          data1 = realloc (data, total_size + 1);
+          if (! data1)
+            goto memory_exhausted;
+          data = data1;
+        }
+
       if (32 < BITS_PER_LONG && *actual_format_ret == 32)
         {
           unsigned long i;
-          int  *idata = (int *) ((*data_ret) + offset);
+          int  *idata = (int *) (data + offset);
           long *ldata = (long *) tmp_data;
 
           for (i = 0; i < *actual_size_ret; ++i)
-            {
-              idata[i]= (int) ldata[i];
-              offset += 4;
-            }
+            idata[i] = ldata[i];
         }
       else
-        {
-          *actual_size_ret *= *actual_format_ret / 8;
-          memcpy ((*data_ret) + offset, tmp_data, *actual_size_ret);
-          offset += *actual_size_ret;
-        }
+        memcpy (data + offset, tmp_data, bytes_gotten);
+
+      offset += bytes_gotten;
 
       /* This was allocated by Xlib, so use XFree.  */
       XFree ((char *) tmp_data);
     }
 
   XFlush (display);
+  data[offset] = '\0';
+
+ done:
   UNBLOCK_INPUT;
+  *data_ret = data;
   *bytes_ret = offset;
+  return;
+
+ size_overflow:
+  free (data);
+  UNBLOCK_INPUT;
+  memory_full (SIZE_MAX);
+
+ memory_exhausted:
+  free (data);
+  UNBLOCK_INPUT;
+  memory_full (total_size + 1);
 }
 
 /* Use xfree, not XFree, to free the data obtained with this function.  */
@@ -1382,16 +1429,19 @@ static void
 receive_incremental_selection (Display *display, Window window, Atom property,
                                Lisp_Object target_type,
                                unsigned int min_size_bytes,
-                               unsigned char **data_ret, int *size_bytes_ret,
+                               unsigned char **data_ret,
+                               ptrdiff_t *size_bytes_ret,
                                Atom *type_ret, int *format_ret,
                                unsigned long *size_ret)
 {
-  int offset = 0;
+  ptrdiff_t offset = 0;
   struct prop_location *wait_object;
+  if (min (PTRDIFF_MAX, SIZE_MAX) < min_size_bytes)
+    memory_full (SIZE_MAX);
+  *data_ret = (unsigned char *) xmalloc (min_size_bytes);
   *size_bytes_ret = min_size_bytes;
-  *data_ret = (unsigned char *) xmalloc (*size_bytes_ret);
 
-  TRACE1 ("Read %d bytes incrementally", min_size_bytes);
+  TRACE1 ("Read %u bytes incrementally", min_size_bytes);
 
   /* At this point, we have read an INCR property.
      Delete the property to ack it.
@@ -1416,7 +1466,7 @@ receive_incremental_selection (Display *display, Window window, Atom property,
   while (1)
     {
       unsigned char *tmp_data;
-      int tmp_size_bytes;
+      ptrdiff_t tmp_size_bytes;
 
       TRACE0 ("  Wait for property change");
       wait_for_property_change (wait_object);
@@ -1429,7 +1479,7 @@ receive_incremental_selection (Display *display, Window window, Atom property,
                              &tmp_data, &tmp_size_bytes,
                              type_ret, format_ret, size_ret, 1);
 
-      TRACE1 ("  Read increment of %d bytes", tmp_size_bytes);
+      TRACE1 ("  Read increment of %"pD"d bytes", tmp_size_bytes);
 
       if (tmp_size_bytes == 0) /* we're done */
         {
@@ -1452,10 +1502,17 @@ receive_incremental_selection (Display *display, Window window, Atom property,
       XFlush (display);
       UNBLOCK_INPUT;
 
-      if (*size_bytes_ret < offset + tmp_size_bytes)
+      if (*size_bytes_ret - offset < tmp_size_bytes)
         {
-          *size_bytes_ret = offset + tmp_size_bytes;
-          *data_ret = (unsigned char *) xrealloc (*data_ret, *size_bytes_ret);
+          ptrdiff_t size;
+          if (min (PTRDIFF_MAX, SIZE_MAX) - offset < tmp_size_bytes)
+            {
+              xfree (tmp_data);
+              memory_full (SIZE_MAX);
+            }
+          size = offset + tmp_size_bytes;
+          *data_ret = (unsigned char *) xrealloc (*data_ret, size);
+          *size_bytes_ret = size;
         }
 
       memcpy ((*data_ret) + offset, tmp_data, tmp_size_bytes);
@@ -1482,7 +1539,7 @@ x_get_window_property_as_lisp_data (Display *display, Window window,
   int actual_format;
   unsigned long actual_size;
   unsigned char *data = 0;
-  int bytes = 0;
+  ptrdiff_t bytes = 0;
   Lisp_Object val;
   struct x_display_info *dpyinfo = x_display_info_for_display (display);
 
@@ -1574,7 +1631,7 @@ x_get_window_property_as_lisp_data (Display *display, Window window,
 
 static Lisp_Object
 selection_data_to_lisp_data (Display *display, const unsigned char *data,
-                             int size, Atom type, int format)
+                             ptrdiff_t size, Atom type, int format)
 {
   struct x_display_info *dpyinfo = x_display_info_for_display (display);
 
@@ -1607,7 +1664,7 @@ selection_data_to_lisp_data (Display *display, const unsigned char *data,
            /* Treat ATOM_PAIR type similar to list of atoms.  */
            || type == dpyinfo->Xatom_ATOM_PAIR)
     {
-      int i;
+      ptrdiff_t i;
       /* On a 64 bit machine sizeof(Atom) == sizeof(long) == 8.
          But the callers of these function has made sure the data for
          format == 32 is an array of int.  Thus, use int instead
@@ -1634,28 +1691,29 @@ selection_data_to_lisp_data (Display *display, const unsigned char *data,
   else if (format == 32 && size == sizeof (int))
     return INTEGER_TO_CONS (((unsigned int *) data) [0]);
   else if (format == 16 && size == sizeof (short))
-    return make_number ((int) (((unsigned short *) data) [0]));
+    return make_number (((unsigned short *) data) [0]);
 
   /* Convert any other kind of data to a vector of numbers, represented
      as above (as an integer, or a cons of two 16 bit integers.)
    */
   else if (format == 16)
     {
-      int i;
+      ptrdiff_t i;
       Lisp_Object v;
       v = Fmake_vector (make_number (size / 2), make_number (0));
       for (i = 0; i < size / 2; i++)
         {
-          int j = (int) ((unsigned short *) data) [i];
+          EMACS_INT j = ((unsigned short *) data) [i];
           Faset (v, make_number (i), make_number (j));
         }
       return v;
     }
   else
     {
-      int i;
-      Lisp_Object v = Fmake_vector (make_number (size / 4), make_number (0));
-      for (i = 0; i < size / 4; i++)
+      ptrdiff_t i;
+      Lisp_Object v = Fmake_vector (make_number (size / X_LONG_SIZE),
+                                    make_number (0));
+      for (i = 0; i < size / X_LONG_SIZE; i++)
         {
           unsigned int j = ((unsigned int *) data) [i];
           Faset (v, make_number (i), INTEGER_TO_CONS (j));
@@ -1670,7 +1728,7 @@ selection_data_to_lisp_data (Display *display, const unsigned char *data,
 static void
 lisp_data_to_selection_data (Display *display, Lisp_Object obj,
                              unsigned char **data_ret, Atom *type_ret,
-                             unsigned int *size_ret,
+                             ptrdiff_t *size_ret,
                              int *format_ret, int *nofree_ret)
 {
   Lisp_Object type = Qnil;
@@ -1707,22 +1765,20 @@ lisp_data_to_selection_data (Display *display, Lisp_Object obj,
     }
   else if (SYMBOLP (obj))
     {
+      *data_ret = (unsigned char *) xmalloc (sizeof (Atom) + 1);
       *format_ret = 32;
       *size_ret = 1;
-      *data_ret = (unsigned char *) xmalloc (sizeof (Atom) + 1);
       (*data_ret) [sizeof (Atom)] = 0;
       (*(Atom **) data_ret) [0] = symbol_to_x_atom (dpyinfo, obj);
       if (NILP (type)) type = QATOM;
     }
-  else if (INTEGERP (obj)
-           && XINT (obj) < 0xFFFF
-           && XINT (obj) > -0xFFFF)
+  else if (RANGED_INTEGERP (0, obj, X_USHRT_MAX))
     {
+      *data_ret = (unsigned char *) xmalloc (sizeof (short) + 1);
       *format_ret = 16;
       *size_ret = 1;
-      *data_ret = (unsigned char *) xmalloc (sizeof (short) + 1);
       (*data_ret) [sizeof (short)] = 0;
-      (*(short **) data_ret) [0] = (short) XINT (obj);
+      (*(unsigned short **) data_ret) [0] = XINT (obj);
       if (NILP (type)) type = QINTEGER;
     }
   else if (INTEGERP (obj)
@@ -1731,11 +1787,11 @@ lisp_data_to_selection_data (Display *display, Lisp_Object obj,
                    || (CONSP (XCDR (obj))
                        && INTEGERP (XCAR (XCDR (obj)))))))
     {
+      *data_ret = (unsigned char *) xmalloc (sizeof (long) + 1);
       *format_ret = 32;
       *size_ret = 1;
-      *data_ret = (unsigned char *) xmalloc (sizeof (long) + 1);
       (*data_ret) [sizeof (long)] = 0;
-      (*(unsigned long **) data_ret) [0] = cons_to_unsigned (obj, ULONG_MAX);
+      (*(unsigned long **) data_ret) [0] = cons_to_unsigned (obj, X_ULONG_MAX);
       if (NILP (type)) type = QINTEGER;
     }
   else if (VECTORP (obj))
@@ -1744,50 +1800,54 @@ lisp_data_to_selection_data (Display *display, Lisp_Object obj,
          a set of 16 or 32 bit INTEGERs;
          or a set of ATOM_PAIRs (represented as [[A1 A2] [A3 A4] ...]
        */
-      int i;
+      ptrdiff_t i;
+      ptrdiff_t size = ASIZE (obj);
 
       if (SYMBOLP (XVECTOR (obj)->contents [0]))
         /* This vector is an ATOM set */
         {
+          if (min (PTRDIFF_MAX, SIZE_MAX) / sizeof (Atom) < size)
+            memory_full (SIZE_MAX);
           if (NILP (type)) type = QATOM;
-          *size_ret = ASIZE (obj);
-          *format_ret = 32;
-          for (i = 0; i < *size_ret; i++)
+          for (i = 0; i < size; i++)
             if (!SYMBOLP (XVECTOR (obj)->contents [i]))
               signal_error ("All elements of selection vector must have same type", obj);
 
-          *data_ret = (unsigned char *) xmalloc ((*size_ret) * sizeof (Atom));
-          for (i = 0; i < *size_ret; i++)
+          *data_ret = (unsigned char *) xmalloc (size * sizeof (Atom));
+          *format_ret = 32;
+          *size_ret = size;
+          for (i = 0; i < size; i++)
             (*(Atom **) data_ret) [i]
               = symbol_to_x_atom (dpyinfo, XVECTOR (obj)->contents [i]);
         }
       else
         /* This vector is an INTEGER set, or something like it */
         {
+          int format = 16;
           int data_size = 2;
-          *size_ret = ASIZE (obj);
           if (NILP (type)) type = QINTEGER;
-          *format_ret = 16;
-          for (i = 0; i < *size_ret; i++)
-            if (CONSP (XVECTOR (obj)->contents [i]))
-              *format_ret = 32;
-            else if (!INTEGERP (XVECTOR (obj)->contents [i]))
-              signal_error (/* Qselection_error */
-    "Elements of selection vector must be integers or conses of integers",
-                            obj);
-
-          /* Use sizeof(long) even if it is more than 32 bits.  See comment
-             in x_get_window_property and x_fill_property_data.  */
-
-          if (*format_ret == 32) data_size = sizeof(long);
-          *data_ret = (unsigned char *) xmalloc (*size_ret * data_size);
-          for (i = 0; i < *size_ret; i++)
-            if (*format_ret == 32)
+          for (i = 0; i < size; i++)
+            if (X_USHRT_MAX
+                < cons_to_unsigned (XVECTOR (obj)->contents[i], X_ULONG_MAX))
+              {
+                /* Use sizeof (long) even if it is more than 32 bits.
+                   See comment in x_get_window_property and
+                   x_fill_property_data.  */
+                data_size = sizeof (long);
+                format = 32;
+              }
+          if (min (PTRDIFF_MAX, SIZE_MAX) / data_size < size)
+            memory_full (SIZE_MAX);
+          *data_ret = (unsigned char *) xmalloc (size * data_size);
+          *format_ret = format;
+          *size_ret = size;
+          for (i = 0; i < size; i++)
+            if (format == 32)
               (*((unsigned long **) data_ret)) [i] =
-                cons_to_unsigned (XVECTOR (obj)->contents [i], ULONG_MAX);
+                cons_to_unsigned (XVECTOR (obj)->contents[i], X_ULONG_MAX);
             else
               (*((unsigned short **) data_ret)) [i] =
-                cons_to_unsigned (XVECTOR (obj)->contents [i], USHRT_MAX);
+                cons_to_unsigned (XVECTOR (obj)->contents[i], X_USHRT_MAX);
         }
     }
   else
@@ -1817,8 +1877,8 @@ clean_local_selection_data (Lisp_Object obj)
     }
   if (VECTORP (obj))
     {
-      int i;
-      int size = ASIZE (obj);
+      ptrdiff_t i;
+      ptrdiff_t size = ASIZE (obj);
       Lisp_Object copy;
       if (size == 1)
         return clean_local_selection_data (XVECTOR (obj)->contents [0]);
@@ -2213,6 +2273,8 @@ x_check_property_data (Lisp_Object data)
       else if (CONSP (o) &&
                (! NUMBERP (XCAR (o)) || ! NUMBERP (XCDR (o))))
         return -1;
+      if (size == INT_MAX)
+        return -1;
       size++;
     }
 
@@ -2294,8 +2356,11 @@ Lisp_Object
 x_property_data_to_lisp (struct frame *f, const unsigned char *data,
                          Atom type, int format, long unsigned int size)
 {
+  ptrdiff_t format_bytes = format >> 3;
+  if (PTRDIFF_MAX / format_bytes < size)
+    memory_full (SIZE_MAX);
   return selection_data_to_lisp_data (FRAME_X_DISPLAY (f),
-                                      data, size*format/8, type, format);
+                                      data, size * format_bytes, type, format);
 }
 
 /* Get the mouse position in frame relative coordinates.  */
@@ -2405,10 +2470,10 @@ FRAME is on.  If FRAME is nil, the selected frame is used.  */)
       if (min (PTRDIFF_MAX, SIZE_MAX) / sizeof *dpyinfo->x_dnd_atoms / 2
           < dpyinfo->x_dnd_atoms_size)
         memory_full (SIZE_MAX);
-      dpyinfo->x_dnd_atoms_size *= 2;
       dpyinfo->x_dnd_atoms = xrealloc (dpyinfo->x_dnd_atoms,
-                                       sizeof (*dpyinfo->x_dnd_atoms)
-                                       * dpyinfo->x_dnd_atoms_size);
+                                       (2 * sizeof *dpyinfo->x_dnd_atoms
+                                        * dpyinfo->x_dnd_atoms_size));
+      dpyinfo->x_dnd_atoms_size *= 2;
     }
 
   dpyinfo->x_dnd_atoms[dpyinfo->x_dnd_atoms_length++] = x_atom;
@@ -2426,7 +2491,7 @@ x_handle_dnd_message (struct frame *f, XClientMessageEvent *event, struct x_disp
   unsigned long size = 160/event->format;
   int x, y;
   unsigned char *data = (unsigned char *) event->data.b;
-  int idata[5];
+  unsigned int idata[5];
   ptrdiff_t i;
 
   for (i = 0; i < dpyinfo->x_dnd_atoms_length; ++i)
@@ -2444,7 +2509,7 @@ x_handle_dnd_message (struct frame *f, XClientMessageEvent *event, struct x_disp
   if (32 < BITS_PER_LONG && event->format == 32)
     {
       for (i = 0; i < 5; ++i) /* There are only 5 longs in a ClientMessage. */
-        idata[i] = (int) event->data.l[i];
+        idata[i] = event->data.l[i];
       data = (unsigned char *) idata;
     }