From: Simon McVittie <simon.mcvittie@collabora.co.uk>
Date: Wed, 16 Feb 2011 17:44:48 +0000 (+0000)
Subject: dbus_message_iter_append_basic: check string-like arguments for validity
X-Git-Tag: dbus-1.4.8~9^2~4^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1d29497d6076c40fed8e151c0b2226e4f86ef62;p=thirdparty%2Fdbus.git

dbus_message_iter_append_basic: check string-like arguments for validity

Strings: UTF-8 with no embedded NULs, by adding a new internal function,
_dbus_check_is_valid_utf8

Object paths, signatures: the obvious syntactic checks

This moves some of the burden of validation to the sender.

When sending <http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-demo.txt>
10240 times with up to 1024 parallel calls pending, on a single-core ARM
Linux device, I found that user CPU time in dbus-spam increased by up to 80%
as a result of the validation. However, when sending messages to dbus-daemon,
overall throughput only reduced by 15%, and when sending messages to an echo
service, overall throughput actually improved by around 14% (presumably
because making the sender CPU-bound influenced kernel scheduling).

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=16338
Bug-NB: NB#223152
Reviewed-by: Cosimo Alfarano <cosimo.alfarano@collabora.co.uk>
---

diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c
index b4579978d..4304467d0 100644
--- a/dbus/dbus-marshal-validate.c
+++ b/dbus/dbus-marshal-validate.c
@@ -1221,6 +1221,8 @@ DEFINE_DBUS_NAME_CHECK(error_name)
 DEFINE_DBUS_NAME_CHECK(bus_name)
 /** define _dbus_check_is_valid_signature() */
 DEFINE_DBUS_NAME_CHECK(signature)
+/** define _dbus_check_is_valid_utf8() */
+DEFINE_DBUS_NAME_CHECK(utf8)
 
 /** @} */
 
diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h
index 8947a2afe..1d2e26b86 100644
--- a/dbus/dbus-marshal-validate.h
+++ b/dbus/dbus-marshal-validate.h
@@ -147,6 +147,8 @@ dbus_bool_t _dbus_validate_bus_name   (const DBusString *str,
 dbus_bool_t _dbus_validate_signature  (const DBusString *str,
                                        int               start,
                                        int               len);
+/* just to have a name consistent with the above: */
+#define _dbus_validate_utf8(s,b,e) _dbus_string_validate_utf8 (s, b, e)
 
 #ifdef DBUS_DISABLE_CHECKS
 
@@ -193,6 +195,8 @@ DECLARE_DBUS_NAME_CHECK(error_name);
 DECLARE_DBUS_NAME_CHECK(bus_name);
 /** defines _dbus_check_is_valid_signature() */
 DECLARE_DBUS_NAME_CHECK(signature);
+/** defines _dbus_check_is_valid_utf8() */
+DECLARE_DBUS_NAME_CHECK(utf8);
 
 /** @} */
 
diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 442ec2ae3..c6b52f552 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -2515,6 +2515,37 @@ dbus_message_iter_append_basic (DBusMessageIter *iter,
   _dbus_return_val_if_fail (dbus_type_is_basic (type), FALSE);
   _dbus_return_val_if_fail (value != NULL, FALSE);
 
+#ifndef DBUS_DISABLE_CHECKS
+  switch (type)
+    {
+      const char * const *string_p;
+
+      case DBUS_TYPE_STRING:
+        string_p = value;
+        _dbus_return_val_if_fail (_dbus_check_is_valid_utf8 (*string_p), FALSE);
+        break;
+
+      case DBUS_TYPE_OBJECT_PATH:
+        string_p = value;
+        _dbus_return_val_if_fail (_dbus_check_is_valid_path (*string_p), FALSE);
+        break;
+
+      case DBUS_TYPE_SIGNATURE:
+        string_p = value;
+        _dbus_return_val_if_fail (_dbus_check_is_valid_signature (*string_p), FALSE);
+        break;
+
+      case DBUS_TYPE_BOOLEAN:
+        /* FIXME: strictly speaking we should ensure that it's in {0,1},
+         * but for now, fall through */
+
+      default:
+          {
+            /* nothing to check, all possible values are allowed */
+          }
+    }
+#endif
+
   if (!_dbus_message_iter_open_signature (real))
     return FALSE;