bug#8722: dbusbind.c fixes for integer issues, e.g., integer overflow

[Paul Eggert]

I found some integer overflow issues in dbusbind.c and came up
with the following patch which I'd like to install.  This doesn't
fix all the integer overflow problems in dbusbind.c, but it makes
some progress anyway.

=== modified file 'src/ChangeLog'
--- src/ChangeLog       2011-05-24 01:20:04 +0000
+++ src/ChangeLog       2011-05-24 05:16:14 +0000
@@ -1,5 +1,23 @@
 2011-05-24  Paul Eggert  <eggert@cs.ucla.edu>
 
+       * dbusbind.c: Serial number integer overflow fixes.
+       (CHECK_DBUS_SERIAL_GET_SERIAL): New macro.
+       (xd_invalid_serial): New static function.
+       (Fdbus_call_method_asynchronously, xd_read_message_1): Use a float
+       to hold a serial number that is too large for a fixnum.
+       (Fdbus_method_return_internal, Fdbus_method_error_internal):
+       Check for serial numbers out of range.  Decode any serial number
+       that was so large that it became a float.
+
+       * dbusbind.c: Use XFASTINT rather than XUINT, and check for nonneg.
+       (Fdbus_call_method, Fdbus_call_method_asynchronously):
+       Use XFASTINT rather than XUINT when numbers are nonnegative.
+       (xd_append_arg, Fdbus_method_return_internal):
+       (Fdbus_method_error_internal): Likewise.  Also, for unsigned
+       arguments, check that Lisp number is nonnegative, rather than
+       silently wrapping negative numbers around.
+       (xd_read_message_1): Don't assume dbus_uint32_t can fit in int.
+
        * data.c (arith_driver, Flsh): Avoid unnecessary casts to EMACS_UINT.
 
 2011-05-23  Paul Eggert  <eggert@cs.ucla.edu>

=== modified file 'src/dbusbind.c'
--- src/dbusbind.c      2011-05-06 22:12:31 +0000
+++ src/dbusbind.c      2011-05-24 05:16:14 +0000
@@ -242,6 +242,31 @@
 #define XD_NEXT_VALUE(object)                                          \
   ((XD_DBUS_TYPE_P (CAR_SAFE (object))) ? CDR_SAFE (object) : object)
 
+/* Check whether X is a valid dbus serial number.  If valid, set
+   SERIAL to its value.  Otherwise, signal an error. */
+#define CHECK_DBUS_SERIAL_GET_SERIAL(x, serial)                                
\
+  do                                                                   \
+    {                                                                  \
+      dbus_uint32_t DBUS_SERIAL_MAX = -1;                              \
+      if (NATNUMP (x) && XINT (x) <= DBUS_SERIAL_MAX)                  \
+       serial = XINT (x);                                              \
+      else if (MOST_POSITIVE_FIXNUM < DBUS_SERIAL_MAX                  \
+              && FLOATP (x)                                            \
+              && 0 <= XFLOAT_DATA (x)                                  \
+              && XFLOAT_DATA (x) <= DBUS_SERIAL_MAX)                   \
+       serial = XFLOAT_DATA (x);                                       \
+      else                                                             \
+       xd_invalid_serial (x);                                          \
+    }                                                                  \
+  while (0)
+
+static void xd_invalid_serial (Lisp_Object) NO_RETURN;
+static void
+xd_invalid_serial (Lisp_Object x)
+{
+  signal_error ("Invalid dbus serial", x);
+}
+
 /* Compute SIGNATURE of OBJECT.  It must have a form that it can be
    used in dbus_message_iter_open_container.  DTYPE is the DBusType
    the object is related to.  It is passed as argument, because it
@@ -431,9 +456,9 @@
     switch (dtype)
       {
       case DBUS_TYPE_BYTE:
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         unsigned char val = XUINT (object) & 0xFF;
+         unsigned char val = XFASTINT (object) & 0xFF;
          XD_DEBUG_MESSAGE ("%c %d", dtype, val);
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
@@ -460,9 +485,9 @@
        }
 
       case DBUS_TYPE_UINT16:
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         dbus_uint16_t val = XUINT (object);
+         dbus_uint16_t val = XFASTINT (object);
          XD_DEBUG_MESSAGE ("%c %u", dtype, (unsigned int) val);
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
@@ -483,9 +508,9 @@
 #ifdef DBUS_TYPE_UNIX_FD
       case DBUS_TYPE_UNIX_FD:
 #endif
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         dbus_uint32_t val = XUINT (object);
+         dbus_uint32_t val = XFASTINT (object);
          XD_DEBUG_MESSAGE ("%c %u", dtype, val);
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
@@ -503,10 +528,10 @@
        }
 
       case DBUS_TYPE_UINT64:
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         dbus_uint64_t val = XUINT (object);
-         XD_DEBUG_MESSAGE ("%c %"pI"u", dtype, XUINT (object));
+         dbus_uint64_t val = XFASTINT (object);
+         XD_DEBUG_MESSAGE ("%c %"pI"d", dtype, XFASTINT (object));
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
          return;
@@ -1110,7 +1135,7 @@
   if ((i+2 <= nargs) && (EQ ((args[i]), QCdbus_timeout)))
     {
       CHECK_NATNUM (args[i+1]);
-      timeout = XUINT (args[i+1]);
+      timeout = XFASTINT (args[i+1]);
       i = i+2;
     }
 
@@ -1186,7 +1211,7 @@
 
   /* Return the result.  If there is only one single Lisp object,
      return it as-it-is, otherwise return the reversed list.  */
-  if (XUINT (Flength (result)) == 1)
+  if (XFASTINT (Flength (result)) == 1)
     RETURN_UNGCPRO (CAR_SAFE (result));
   else
     RETURN_UNGCPRO (Fnreverse (result));
@@ -1251,6 +1276,7 @@
   DBusMessage *dmessage;
   DBusMessageIter iter;
   unsigned int dtype;
+  dbus_uint32_t serial;
   int timeout = -1;
   size_t i = 6;
   char signature[DBUS_MAXIMUM_SIGNATURE_LENGTH];
@@ -1292,7 +1318,7 @@
   if ((i+2 <= nargs) && (EQ ((args[i]), QCdbus_timeout)))
     {
       CHECK_NATNUM (args[i+1]);
-      timeout = XUINT (args[i+1]);
+      timeout = XFASTINT (args[i+1]);
       i = i+2;
     }
 
@@ -1335,7 +1361,8 @@
        XD_SIGNAL1 (build_string ("Cannot send message"));
 
       /* The result is the key in Vdbus_registered_objects_table.  */
-      result = (list2 (bus, make_number (dbus_message_get_serial (dmessage))));
+      serial = dbus_message_get_serial (dmessage);
+      result = list2 (bus, make_fixnum_or_float (serial));
 
       /* Create a hash table entry.  */
       Fputhash (result, handler, Vdbus_registered_objects_table);
@@ -1368,25 +1395,26 @@
 usage: (dbus-method-return-internal BUS SERIAL SERVICE &rest ARGS)  */)
   (size_t nargs, register Lisp_Object *args)
 {
-  Lisp_Object bus, serial, service;
-  struct gcpro gcpro1, gcpro2, gcpro3;
+  Lisp_Object bus, service;
+  struct gcpro gcpro1, gcpro2;
   DBusConnection *connection;
   DBusMessage *dmessage;
   DBusMessageIter iter;
-  unsigned int dtype;
+  dbus_uint32_t serial;
+  unsigned int ui_serial, dtype;
   size_t i;
   char signature[DBUS_MAXIMUM_SIGNATURE_LENGTH];
 
   /* Check parameters.  */
   bus = args[0];
-  serial = args[1];
   service = args[2];
 
-  CHECK_NUMBER (serial);
+  CHECK_DBUS_SERIAL_GET_SERIAL (args[1], serial);
   CHECK_STRING (service);
-  GCPRO3 (bus, serial, service);
+  GCPRO2 (bus, service);
 
-  XD_DEBUG_MESSAGE ("%"pI"u %s ", XUINT (serial), SDATA (service));
+  ui_serial = serial;
+  XD_DEBUG_MESSAGE ("%u %s ", ui_serial, SSDATA (service));
 
   /* Open a connection to the bus.  */
   connection = xd_initialize (bus, TRUE);
@@ -1394,7 +1422,7 @@
   /* Create the message.  */
   dmessage = dbus_message_new (DBUS_MESSAGE_TYPE_METHOD_RETURN);
   if ((dmessage == NULL)
-      || (!dbus_message_set_reply_serial (dmessage, XUINT (serial)))
+      || (!dbus_message_set_reply_serial (dmessage, serial))
       || (!dbus_message_set_destination (dmessage, SSDATA (service))))
     {
       UNGCPRO;
@@ -1456,25 +1484,26 @@
 usage: (dbus-method-error-internal BUS SERIAL SERVICE &rest ARGS)  */)
   (size_t nargs, register Lisp_Object *args)
 {
-  Lisp_Object bus, serial, service;
-  struct gcpro gcpro1, gcpro2, gcpro3;
+  Lisp_Object bus, service;
+  struct gcpro gcpro1, gcpro2;
   DBusConnection *connection;
   DBusMessage *dmessage;
   DBusMessageIter iter;
-  unsigned int dtype;
+  dbus_uint32_t serial;
+  unsigned int ui_serial, dtype;
   size_t i;
   char signature[DBUS_MAXIMUM_SIGNATURE_LENGTH];
 
   /* Check parameters.  */
   bus = args[0];
-  serial = args[1];
   service = args[2];
 
-  CHECK_NUMBER (serial);
+  CHECK_DBUS_SERIAL_GET_SERIAL (args[1], serial);
   CHECK_STRING (service);
-  GCPRO3 (bus, serial, service);
+  GCPRO2 (bus, service);
 
-  XD_DEBUG_MESSAGE ("%"pI"u %s ", XUINT (serial), SDATA (service));
+  ui_serial = serial;
+  XD_DEBUG_MESSAGE ("%u %s ", ui_serial, SSDATA (service));
 
   /* Open a connection to the bus.  */
   connection = xd_initialize (bus, TRUE);
@@ -1483,7 +1512,7 @@
   dmessage = dbus_message_new (DBUS_MESSAGE_TYPE_ERROR);
   if ((dmessage == NULL)
       || (!dbus_message_set_error_name (dmessage, DBUS_ERROR_FAILED))
-      || (!dbus_message_set_reply_serial (dmessage, XUINT (serial)))
+      || (!dbus_message_set_reply_serial (dmessage, serial))
       || (!dbus_message_set_destination (dmessage, SSDATA (service))))
     {
       UNGCPRO;
@@ -1663,7 +1692,9 @@
   DBusMessage *dmessage;
   DBusMessageIter iter;
   unsigned int dtype;
-  int mtype, serial;
+  int mtype;
+  dbus_uint32_t serial;
+  unsigned int ui_serial;
   const char *uname, *path, *interface, *member;
 
   dmessage = dbus_connection_pop_message (connection);
@@ -1692,7 +1723,7 @@
   /* Read message type, message serial, unique name, object path,
      interface and member from the message.  */
   mtype = dbus_message_get_type (dmessage);
-  serial =
+  ui_serial = serial =
     ((mtype == DBUS_MESSAGE_TYPE_METHOD_RETURN)
      || (mtype == DBUS_MESSAGE_TYPE_ERROR))
     ? dbus_message_get_reply_serial (dmessage)
@@ -1702,7 +1733,7 @@
   interface = dbus_message_get_interface (dmessage);
   member = dbus_message_get_member (dmessage);
 
-  XD_DEBUG_MESSAGE ("Event received: %s %d %s %s %s %s %s",
+  XD_DEBUG_MESSAGE ("Event received: %s %u %s %s %s %s %s",
                    (mtype == DBUS_MESSAGE_TYPE_INVALID)
                    ? "DBUS_MESSAGE_TYPE_INVALID"
                    : (mtype == DBUS_MESSAGE_TYPE_METHOD_CALL)
@@ -1712,14 +1743,14 @@
                    : (mtype == DBUS_MESSAGE_TYPE_ERROR)
                    ? "DBUS_MESSAGE_TYPE_ERROR"
                    : "DBUS_MESSAGE_TYPE_SIGNAL",
-                   serial, uname, path, interface, member,
+                   ui_serial, uname, path, interface, member,
                    SDATA (format2 ("%s", args, Qnil)));
 
   if ((mtype == DBUS_MESSAGE_TYPE_METHOD_RETURN)
       || (mtype == DBUS_MESSAGE_TYPE_ERROR))
     {
       /* Search for a registered function of the message.  */
-      key = list2 (bus, make_number (serial));
+      key = list2 (bus, make_fixnum_or_float (serial));
       value = Fgethash (key, Vdbus_registered_objects_table, Qnil);
 
       /* There shall be exactly one entry.  Construct an event.  */
@@ -1785,7 +1816,7 @@
                     event.arg);
   event.arg = Fcons ((uname == NULL ? Qnil : build_string (uname)),
                     event.arg);
-  event.arg = Fcons (make_number (serial), event.arg);
+  event.arg = Fcons (make_fixnum_or_float (serial), event.arg);
   event.arg = Fcons (make_number (mtype), event.arg);
 
   /* Add the bus symbol to the event.  */