core: refactor nm_ethernet_address_is_valid() and reject invalid addresses

nm_ethernet_address_is_valid() did not check whether @addr was a valid
address in the first place. It only checked whether the address was not
equal to a few notorious MAC addresses.

At the same time, be more forgiving and accept %NULL as argument.
This fixes an assertion nm_ap_match_in_hash().

2 files changed, 28 insertions, 25 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index 3153edc77c..a5de386652 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -71,36 +71,36 @@
 gboolean
 nm_ethernet_address_is_valid (gconstpointer addr, gssize len)
 {
-	guint8 invalid_addr1[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
-	guint8 invalid_addr2[ETH_ALEN] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
-	guint8 invalid_addr3[ETH_ALEN] = {0x44, 0x44, 0x44, 0x44, 0x44, 0x44};
-	guint8 invalid_addr4[ETH_ALEN] = {0x00, 0x30, 0xb4, 0x00, 0x00, 0x00}; /* prism54 dummy MAC */
-	guchar first_octet;
-
-	g_return_val_if_fail (addr != NULL, FALSE);
-	g_return_val_if_fail (len == ETH_ALEN || len == -1, FALSE);
-
-	/* Compare the AP address the card has with invalid ethernet MAC addresses. */
-	if (nm_utils_hwaddr_matches (addr, len, invalid_addr1, ETH_ALEN))
-		return FALSE;
+	guint8 invalid_addr[4][ETH_ALEN] = {
+	    {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF},
+	    {0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+	    {0x44, 0x44, 0x44, 0x44, 0x44, 0x44},
+	    {0x00, 0x30, 0xb4, 0x00, 0x00, 0x00}, /* prism54 dummy MAC */
+	};
+	guint8 addr_bin[ETH_ALEN];
+	guint i;
 
-	if (nm_utils_hwaddr_matches (addr, len, invalid_addr2, ETH_ALEN))
+	if (!addr) {
+		g_return_val_if_fail (len == -1 || len == ETH_ALEN, FALSE);
 		return FALSE;
+	}
 
-	if (nm_utils_hwaddr_matches (addr, len, invalid_addr3, ETH_ALEN))
-		return FALSE;
+	if (len == -1) {
+		if (!nm_utils_hwaddr_aton (addr, addr_bin, ETH_ALEN))
+			return FALSE;
+		addr = addr_bin;
+	} else if (len != ETH_ALEN)
+		g_return_val_if_reached (FALSE);
 
-	if (nm_utils_hwaddr_matches (addr, len, invalid_addr4, ETH_ALEN))
+	/* Check for multicast address */
+	if ((((guint8 *) addr)[0]) & 0x01)
 		return FALSE;
 
-	/* Check for multicast address */
-	if (len == -1)
-		first_octet = strtoul (addr, NULL, 16);
-	else
-		first_octet = ((guint8 *)addr)[0];
-	if (first_octet & 0x01)
+	for (i = 0; i < G_N_ELEMENTS (invalid_addr); i++) {
+		if (nm_utils_hwaddr_matches (addr, ETH_ALEN, invalid_addr[i], ETH_ALEN))
 			return FALSE;
-	
+	}
+
 	return TRUE;
 }
 
diff --git a/src/tests/test-general-with-expect.c b/src/tests/test-general-with-expect.c
index 4d7548592e..4318b594c6 100644
--- a/src/tests/test-general-with-expect.c
+++ b/src/tests/test-general-with-expect.c
@@ -434,14 +434,17 @@ test_nm_utils_array_remove_at_indexes ()
 static void
 test_nm_ethernet_address_is_valid ()
 {
+	g_assert (!nm_ethernet_address_is_valid (NULL, -1));
+	g_assert (!nm_ethernet_address_is_valid (NULL, ETH_ALEN));
+
 	g_assert (!nm_ethernet_address_is_valid ("FF:FF:FF:FF:FF:FF", -1));
 	g_assert (!nm_ethernet_address_is_valid ("00:00:00:00:00:00", -1));
 	g_assert (!nm_ethernet_address_is_valid ("44:44:44:44:44:44", -1));
 	g_assert (!nm_ethernet_address_is_valid ("00:30:b4:00:00:00", -1));
 
-	g_assert ( nm_ethernet_address_is_valid ("", -1));
+	g_assert (!nm_ethernet_address_is_valid ("", -1));
 	g_assert (!nm_ethernet_address_is_valid ("1", -1));
-	g_assert ( nm_ethernet_address_is_valid ("2", -1));
+	g_assert (!nm_ethernet_address_is_valid ("2", -1));
 
 	g_assert (!nm_ethernet_address_is_valid (((guint8[8]) { 0x00,0x30,0xb4,0x00,0x00,0x00 }), ETH_ALEN));
 	g_assert ( nm_ethernet_address_is_valid (((guint8[8]) { 0x00,0x30,0xb4,0x00,0x00,0x01 }), ETH_ALEN));