libnm: don't compare invalid mac addresses as equal in nm_utils_hwaddr_matches()

By passing as length of the MAC addresses -1 for both arguments, one
could get through to compare empty strings, NULL, and addresses longer
than the maximum. Such addresses are not valid, and they should never
compare equal (not even to themselves).

This is a change in behavior of public API, but it never made sense to
claim two addresses are equal, when they are not even valid addresses.

Also, avoid undefined behavior with "NULL, -1, NULL, -1" arguments,
where we would call memcmp() with zero length and NULL arguments.
UBSan flags that too.

(cherry picked from commit 54a64edefc)
This commit is contained in:
Thomas Haller 2020-05-13 22:48:34 +02:00
parent 2c9af9db43
commit 3a66217cbb
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 13 additions and 4 deletions

View file

@ -4269,7 +4269,8 @@ nm_utils_hwaddr_matches (gconstpointer hwaddr1,
hwaddr1 = buf1;
hwaddr1_len = l;
} else {
g_return_val_if_fail ((hwaddr2_len == -1 && hwaddr2) || (hwaddr2_len > 0 && hwaddr2_len <= NM_UTILS_HWADDR_LEN_MAX), FALSE);
g_return_val_if_fail ( hwaddr2_len == -1
|| (hwaddr2_len > 0 && hwaddr2_len <= NM_UTILS_HWADDR_LEN_MAX), FALSE);
return FALSE;
}
} else {
@ -4301,9 +4302,17 @@ nm_utils_hwaddr_matches (gconstpointer hwaddr1,
}
}
if (G_UNLIKELY ( hwaddr1_len <= 0
|| hwaddr1_len > NM_UTILS_HWADDR_LEN_MAX)) {
/* Only valid addresses can compare equal. In particular,
* addresses that are too long or of zero bytes, never
* compare equal. */
return FALSE;
}
if (hwaddr1_len == INFINIBAND_ALEN) {
hwaddr1 = (guint8 *)hwaddr1 + INFINIBAND_ALEN - 8;
hwaddr2 = (guint8 *)hwaddr2 + INFINIBAND_ALEN - 8;
hwaddr1 = &((guint8 *) hwaddr1)[INFINIBAND_ALEN - 8];
hwaddr2 = &((guint8 *) hwaddr2)[INFINIBAND_ALEN - 8];
hwaddr1_len = 8;
}

View file

@ -4277,7 +4277,7 @@ test_hwaddr_equal (void)
g_assert (nm_utils_hwaddr_matches (null_binary, sizeof (null_binary), null_binary, sizeof (null_binary)));
g_assert (nm_utils_hwaddr_matches (null_binary, sizeof (null_binary), NULL, ETH_ALEN));
g_assert (nm_utils_hwaddr_matches (NULL, -1, NULL, -1));
g_assert (!nm_utils_hwaddr_matches (NULL, -1, NULL, -1));
g_assert (!nm_utils_hwaddr_matches (NULL, -1, string, -1));
g_assert (!nm_utils_hwaddr_matches (string, -1, NULL, -1));
g_assert (!nm_utils_hwaddr_matches (NULL, -1, null_string, -1));