From e5c1be89b5fbf5bcdb645314265675fd47d64774 Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Fri, 4 May 2018 17:36:40 +0900
Subject: [PATCH] ether-addr-util: make ether_addr_from_string() stricter

---
 src/basic/ether-addr-util.c               | 24 +++++++++++++----------
 src/basic/ether-addr-util.h               |  2 +-
 src/libsystemd-network/network-internal.c | 17 ++++++----------
 src/network/test-networkd-conf.c          |  9 +++++----
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/basic/ether-addr-util.c b/src/basic/ether-addr-util.c
index e6ca7fa526..7e08ba09f0 100644
--- a/src/basic/ether-addr-util.c
+++ b/src/basic/ether-addr-util.c
@@ -45,7 +45,7 @@ bool ether_addr_equal(const struct ether_addr *a, const struct ether_addr *b) {
                 a->ether_addr_octet[5] == b->ether_addr_octet[5];
 }
 
-int ether_addr_from_string(const char *s, struct ether_addr *ret, size_t *offset) {
+int ether_addr_from_string(const char *s, struct ether_addr *ret) {
         size_t pos = 0, n, field;
         char sep = '\0';
         const char *hex = HEXDIGITS, *hexoff;
@@ -84,31 +84,35 @@ int ether_addr_from_string(const char *s, struct ether_addr *ret, size_t *offset
         assert(s);
         assert(ret);
 
+        s += strspn(s, WHITESPACE);
         sep = s[strspn(s, hex)];
-        if (sep == '\n')
-                return -EINVAL;
-        if (!strchr(":.-", sep))
-                return -EINVAL;
 
         if (sep == '.') {
                 uint16_t shorts[3] = { 0 };
 
                 parse_fields(shorts);
 
+                if (s[pos] != '\0')
+                        return -EINVAL;
+
                 for (n = 0; n < ELEMENTSOF(shorts); n++) {
                         ret->ether_addr_octet[2*n] = ((shorts[n] & (uint16_t)0xff00) >> 8);
                         ret->ether_addr_octet[2*n + 1] = (shorts[n] & (uint16_t)0x00ff);
                 }
-        } else {
-                struct ether_addr out = { .ether_addr_octet = { 0 } };
+
+        } else if (IN_SET(sep, ':', '-')) {
+                struct ether_addr out = ETHER_ADDR_NULL;
 
                 parse_fields(out.ether_addr_octet);
 
+                if (s[pos] != '\0')
+                        return -EINVAL;
+
                 for (n = 0; n < ELEMENTSOF(out.ether_addr_octet); n++)
                         ret->ether_addr_octet[n] = out.ether_addr_octet[n];
-        }
 
-        if (offset)
-                *offset = pos;
+        } else
+                return -EINVAL;
+
         return 0;
 }
diff --git a/src/basic/ether-addr-util.h b/src/basic/ether-addr-util.h
index 29d7f36294..11ff72a235 100644
--- a/src/basic/ether-addr-util.h
+++ b/src/basic/ether-addr-util.h
@@ -24,4 +24,4 @@ static inline bool ether_addr_is_null(const struct ether_addr *addr) {
         return ether_addr_equal(addr, &ETHER_ADDR_NULL);
 }
 
-int ether_addr_from_string(const char *s, struct ether_addr *ret, size_t *offset);
+int ether_addr_from_string(const char *s, struct ether_addr *ret);
diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c
index 620f711545..9a11c0d975 100644
--- a/src/libsystemd-network/network-internal.c
+++ b/src/libsystemd-network/network-internal.c
@@ -281,10 +281,9 @@ int config_parse_hwaddr(const char *unit,
                         const char *rvalue,
                         void *data,
                         void *userdata) {
+
+        _cleanup_free_ struct ether_addr *n = NULL;
         struct ether_addr **hwaddr = data;
-        struct ether_addr *n;
-        const char *start;
-        size_t offset;
         int r;
 
         assert(filename);
@@ -296,17 +295,13 @@ int config_parse_hwaddr(const char *unit,
         if (!n)
                 return log_oom();
 
-        start = rvalue + strspn(rvalue, WHITESPACE);
-        r = ether_addr_from_string(start, n, &offset);
-
-        if (r || (start[offset + strspn(start + offset, WHITESPACE)] != '\0')) {
-                log_syntax(unit, LOG_ERR, filename, line, 0, "Not a valid MAC address, ignoring assignment: %s", rvalue);
-                free(n);
+        r = ether_addr_from_string(rvalue, n);
+        if (r < 0) {
+                log_syntax(unit, LOG_ERR, filename, line, r, "Not a valid MAC address, ignoring assignment: %s", rvalue);
                 return 0;
         }
 
-        free(*hwaddr);
-        *hwaddr = n;
+        *hwaddr = TAKE_PTR(n);
 
         return 0;
 }
diff --git a/src/network/test-networkd-conf.c b/src/network/test-networkd-conf.c
index b966fbd960..e24fea9c80 100644
--- a/src/network/test-networkd-conf.c
+++ b/src/network/test-networkd-conf.c
@@ -58,10 +58,10 @@ static void test_config_parse_hwaddr_one(const char *rvalue, int ret, const stru
         assert_se(ret == r);
         if (expected) {
                 assert_se(actual);
-                assert(ether_addr_equal(expected, actual));
-        } else {
+                assert_se(ether_addr_equal(expected, actual));
+        } else
                 assert_se(actual == NULL);
-        }
+
         free(actual);
 }
 
@@ -90,12 +90,13 @@ static void test_config_parse_hwaddr(void) {
                 { .ether_addr_octet = { 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff } },
                 { .ether_addr_octet = { 0x01, 0x23, 0x45, 0x67, 0x89, 0xab } },
         };
+
         test_config_parse_hwaddr_one("", 0, NULL);
         test_config_parse_hwaddr_one("no:ta:ma:ca:dd:re", 0, NULL);
         test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:fx", 0, NULL);
         test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff", 0, &t[0]);
         test_config_parse_hwaddr_one(" aa:bb:cc:dd:ee:ff", 0, &t[0]);
-        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff \t\n", 0, &t[0]);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff \t\n", 0, NULL);
         test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff \t\nxxx", 0, NULL);
         test_config_parse_hwaddr_one("aa:bb:cc: dd:ee:ff", 0, NULL);
         test_config_parse_hwaddr_one("aa:bb:cc:d d:ee:ff", 0, NULL);
-- 
2.25.1