diff options
| author | Thomas Haller <thaller@redhat.com> | 2020-05-13 22:48:34 +0200 |
|---|---|---|
| committer | Thomas Haller <thaller@redhat.com> | 2020-05-14 11:06:09 +0200 |
| commit | 54a64edefc4222b8ef20d837c885618ea5f6e0d7 (patch) | |
| tree | 08d49f85ff91d388dff94394f0cfd09a0d81bb94 | |
| parent | 3c581cbb78fb805856ec8ce03bcb993cca0d93d8 (diff) | |
| download | NetworkManager-54a64edefc4222b8ef20d837c885618ea5f6e0d7.tar.gz | |
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.
| -rw-r--r-- | libnm-core/nm-utils.c | 15 | ||||
| -rw-r--r-- | libnm-core/tests/test-general.c | 2 |
2 files changed, 13 insertions, 4 deletions
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index f9bccea930..59798c1d9b 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -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; } diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 52f05dc306..a6fb700715 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -4278,7 +4278,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)); |