]> git.ipfire.org Git - thirdparty/dbus.git/commitdiff
keyring: Avoid undefined out-of-range shift
authorSimon McVittie <smcv@collabora.com>
Fri, 17 Aug 2018 18:38:07 +0000 (19:38 +0100)
committerSimon McVittie <smcv@collabora.com>
Mon, 19 Nov 2018 12:15:17 +0000 (12:15 +0000)
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>
dbus/dbus-keyring.c

index dbeb8791d30b9af6567ee7e1af255a2e0cb267d1..28f489e418c095969c9b4f56b3b5eb3a98f1e213 100644 (file)
@@ -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)