From 70fdc001be8b99d4d1f0b5ee93b8b59c978d66be Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 17 Aug 2018 19:38:07 +0100 Subject: [PATCH] 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 --- dbus/dbus-keyring.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c index dbeb8791..28f489e4 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)