From: Simon McVittie <smcv@collabora.com>
Date: Fri, 17 Aug 2018 18:38:07 +0000 (+0100)
Subject: keyring: Avoid undefined out-of-range shift
X-Git-Tag: dbus-1.13.8~27^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=70fdc001be8b99d4d1f0b5ee93b8b59c978d66be;p=thirdparty%2Fdbus.git

keyring: Avoid undefined out-of-range shift

Detected with UndefinedBehaviourSanitizer, which will warn on
about 50% of calls to this function, when s[3] is 128 or more,
because id is signed, so 128 << 24 is undefined signed overflow.

All we want here is a random non-negative signed int (in the range 0
to 2**31-1, with 31 bits varying). The intention seemed to be to
generate a random unsigned int, cast it to signed, and then negate it
if negative, but it seems simpler and more obviously correct to just
make sure the most  significant byte fits in the non-negative range.

Signed-off-by: Simon McVittie <smcv@collabora.com>
---

diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
index dbeb8791d..28f489e41 100644
--- a/dbus/dbus-keyring.c
+++ b/dbus/dbus-keyring.c
@@ -310,9 +310,7 @@ add_new_key (DBusKey  **keys_p,
 
   s = (const unsigned char*) _dbus_string_get_const_data (&bytes);
       
-  id = s[0] | (s[1] << 8) | (s[2] << 16) | (s[3] << 24);
-  if (id < 0)
-    id = - id;
+  id = s[0] | (s[1] << 8) | (s[2] << 16) | ((s[3] & 0x7f) << 24);
   _dbus_assert (id >= 0);
 
   if (find_key_by_id (keys, n_keys, id) != NULL)