dhcp6: Sanitize DHCPv6 IA option parsing

Sanitize code for parsing DHCPv6 IA NA and TA options and their
nested Status options so that the options can be fully and
properly ignored should they not be conformant to the specification.

Do this by defining a proper DHCP6Option structure and sending that
structure to the parsing function. The parsing function will then
not manipulate either any option data pointers or their lengths in
order to iterate over the current option. Needless to say, this
affects a few files including the test program.

diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h
index 9bff5c2dcd97be70d70c5a18b37872f19d19704e..9e7d3976aa38a342d78b467ceee9d59607b7c548 100644 (file)
--- a/src/libsystemd-network/dhcp6-internal.h
+++ b/src/libsystemd-network/dhcp6-internal.h
@@ -29,6 +29,13 @@
 #include "macro.h"
 #include "sparse-endian.h"
 
+/* Common option header */
+typedef struct DHCP6Option {
+        be16_t code;
+        be16_t len;
+        uint8_t data[];
+} _packed_ DHCP6Option;
+
 typedef struct DHCP6Address DHCP6Address;
 
 struct DHCP6Address {
@@ -76,8 +83,7 @@ int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, DHCP6IA *ia);
 int dhcp6_option_append_fqdn(uint8_t **buf, size_t *buflen, const char *fqdn);
 int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode,
                        size_t *optlen, uint8_t **optvalue);
-int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
-                          DHCP6IA *ia);
+int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia);
 int dhcp6_option_parse_ip6addrs(uint8_t *optval, uint16_t optlen,
                                 struct in6_addr **addrs, size_t count,
                                 size_t *allocated);

diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c
index 139b7a4c5c5e41aca2fef2fe3c340d101a96b8cd..80925b3948ca1e5544c5d239fab138484ecdee27 100644 (file)
--- a/src/libsystemd-network/dhcp6-option.c
+++ b/src/libsystemd-network/dhcp6-option.c
@@ -36,12 +36,6 @@
 #define DHCP6_OPTION_IA_NA_LEN                  12
 #define DHCP6_OPTION_IA_TA_LEN                  4
 
-typedef struct DHCP6Option {
-        be16_t code;
-        be16_t len;
-        uint8_t data[];
-} _packed_ DHCP6Option;
-
 static int option_append_hdr(uint8_t **buf, size_t *buflen, uint16_t optcode,
                              size_t optlen) {
         DHCP6Option *option = (DHCP6Option*) *buf;
@@ -213,11 +207,11 @@ int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode,
         return 0;
 }
 
-int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
-                          DHCP6IA *ia) {
-        int r;
-        uint16_t opt, status;
-        size_t optlen;
+int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia) {
+        uint16_t iatype, optlen;
+        size_t i, len;
+        int r = 0, status;
+        uint16_t opt;
         size_t iaaddr_offset;
         DHCP6Address *addr;
         uint32_t lt_t1, lt_t2, lt_valid, lt_pref, lt_min = ~0;
@@ -225,17 +219,19 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
         assert_return(ia, -EINVAL);
         assert_return(!ia->addresses, -EINVAL);
 
+        iatype = be16toh(iaoption->code);
+        len = be16toh(iaoption->len);
+
         switch (iatype) {
         case SD_DHCP6_OPTION_IA_NA:
 
-                if (*buflen < DHCP6_OPTION_IA_NA_LEN + sizeof(DHCP6Option) +
-                    sizeof(addr->iaaddr)) {
+                if (len < DHCP6_OPTION_IA_NA_LEN) {
                         r = -ENOBUFS;
                         goto error;
                 }
 
                 iaaddr_offset = DHCP6_OPTION_IA_NA_LEN;
-                memcpy(&ia->ia_na, *buf, sizeof(ia->ia_na));
+                memcpy(&ia->ia_na, iaoption->data, sizeof(ia->ia_na));
 
                 lt_t1 = be32toh(ia->ia_na.lifetime_t1);
                 lt_t2 = be32toh(ia->ia_na.lifetime_t2);
@@ -250,14 +246,13 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
                 break;
 
         case SD_DHCP6_OPTION_IA_TA:
-                if (*buflen < DHCP6_OPTION_IA_TA_LEN + sizeof(DHCP6Option) +
-                    sizeof(addr->iaaddr)) {
+                if (len < DHCP6_OPTION_IA_TA_LEN) {
                         r = -ENOBUFS;
                         goto error;
                 }
 
                 iaaddr_offset = DHCP6_OPTION_IA_TA_LEN;
-                memcpy(&ia->ia_ta.id, *buf, sizeof(ia->ia_ta));
+                memcpy(&ia->ia_ta.id, iaoption->data, sizeof(ia->ia_ta));
 
                 break;
 
@@ -267,15 +262,21 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
         }
 
         ia->type = iatype;
+        i = iaaddr_offset;
+
+        while (i < len) {
+                DHCP6Option *option = (DHCP6Option *)&iaoption->data[i];
 
-        *buflen -= iaaddr_offset;
-        *buf += iaaddr_offset;
+                if (len < i + sizeof(*option) || len < i + sizeof(*option) + be16toh(option->len)) {
+                        r = -ENOBUFS;
+                        goto error;
+                }
 
-        while ((r = option_parse_hdr(buf, buflen, &opt, &optlen)) >= 0) {
+                opt = be16toh(option->code);
+                optlen = be16toh(option->len);
 
                 switch (opt) {
                 case SD_DHCP6_OPTION_IAADDR:
-
                         addr = new0(DHCP6Address, 1);
                         if (!addr) {
                                 r = -ENOMEM;
@@ -283,8 +284,7 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
                         }
 
                         LIST_INIT(addresses, addr);
-
-                        memcpy(&addr->iaaddr, *buf, sizeof(addr->iaaddr));
+                        memcpy(&addr->iaaddr, option->data, sizeof(addr->iaaddr));
 
                         lt_valid = be32toh(addr->iaaddr.lifetime_valid);
                         lt_pref = be32toh(addr->iaaddr.lifetime_valid);
@@ -302,10 +302,12 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
                         break;
 
                 case SD_DHCP6_OPTION_STATUS_CODE:
-                        if (optlen < sizeof(status))
-                                break;
+                        if (optlen < sizeof(status)) {
+                                r = -ENOMSG;
+                                goto error;
+                        }
 
-                        status = (*buf)[0] << 8 | (*buf)[1];
+                        status = option->data[0] << 8 | option->data[1];
                         if (status) {
                                 log_dhcp6_client(client, "IA status %d",
                                                  status);
@@ -320,16 +322,9 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
                         break;
                 }
 
-                *buflen -= optlen;
-                *buf += optlen;
+                i += sizeof(*option) + optlen;
         }
 
-        if (r == -ENOMSG)
-                r = 0;
-
-        if (*buflen)
-                r = -ENOMSG;
-
         if (!ia->ia_na.lifetime_t1 && !ia->ia_na.lifetime_t2) {
                 lt_t1 = lt_min / 2;
                 lt_t2 = lt_min / 10 * 8;
@@ -341,9 +336,6 @@ int dhcp6_option_parse_ia(uint8_t **buf, size_t *buflen, uint16_t iatype,
         }
 
 error:
-        *buf += *buflen;
-        *buflen = 0;
-
         return r;
 }
 

diff --git a/src/libsystemd-network/dhcp6-protocol.h b/src/libsystemd-network/dhcp6-protocol.h
index 7bbf183996f43e0680e9704fc5cd727ab53a3371..5f7e809ba1a28fb638c13488da3fda3a5937003f 100644 (file)
--- a/src/libsystemd-network/dhcp6-protocol.h
+++ b/src/libsystemd-network/dhcp6-protocol.h
@@ -34,6 +34,7 @@ struct DHCP6Message {
                 } _packed_;
                 be32_t transaction_id;
         };
+        uint8_t options[];
 } _packed_;
 
 typedef struct DHCP6Message DHCP6Message;

diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c
index 87f37dc0d46a9b16a81962aaf8f1d3f1a3b22a8b..9c758a4d0bc00d797d347d2ce95ac578f9df4550 100644 (file)
--- a/src/libsystemd-network/sd-dhcp6-client.c
+++ b/src/libsystemd-network/sd-dhcp6-client.c
@@ -725,23 +725,32 @@ static int client_parse_message(
                 DHCP6Message *message,
                 size_t len,
                 sd_dhcp6_lease *lease) {
+        size_t pos = 0;
         int r;
-        uint8_t *optval, *option, *id = NULL;
-        uint16_t optcode, status;
-        size_t optlen, id_len;
         bool clientid = false;
-        be32_t iaid_lease;
+        uint8_t *id = NULL;
+        size_t id_len;
 
         assert(client);
         assert(message);
         assert(len >= sizeof(DHCP6Message));
         assert(lease);
 
-        option = (uint8_t *)message + sizeof(DHCP6Message);
         len -= sizeof(DHCP6Message);
 
-        while ((r = dhcp6_option_parse(&option, &len, &optcode, &optlen,
-                                       &optval)) >= 0) {
+        while (pos < len) {
+                DHCP6Option *option = (DHCP6Option *)&message->options[pos];
+                uint16_t optcode, optlen, status;
+                uint8_t *optval;
+                be32_t iaid_lease;
+
+                if (len < sizeof(DHCP6Option) || len < sizeof(DHCP6Option) + be16toh(option->len))
+                        return -ENOBUFS;
+
+                optcode = be16toh(option->code);
+                optlen = be16toh(option->len);
+                optval = option->data;
+
                 switch (optcode) {
                 case SD_DHCP6_OPTION_CLIENTID:
                         if (clientid) {
@@ -779,7 +788,7 @@ static int client_parse_message(
                         if (optlen != 1)
                                 return -EINVAL;
 
-                        r = dhcp6_lease_set_preference(lease, *optval);
+                        r = dhcp6_lease_set_preference(lease, optval[0]);
                         if (r < 0)
                                 return r;
 
@@ -806,8 +815,7 @@ static int client_parse_message(
                                 break;
                         }
 
-                        r = dhcp6_option_parse_ia(&optval, &optlen, optcode,
-                                                  &lease->ia);
+                        r = dhcp6_option_parse_ia(option, &lease->ia);
                         if (r < 0 && r != -ENOMSG)
                                 return r;
 
@@ -859,11 +867,9 @@ static int client_parse_message(
                         break;
                 }
 
+                pos += sizeof(*option) + optlen;
         }
 
-        if (r == -ENOMSG)
-                r = 0;
-
         if (r < 0 || !clientid) {
                 log_dhcp6_client(client, "%s has incomplete options",
                                  dhcp6_message_type_to_string(message->type));

diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c
index a0418ecdb97614e4725602360a6ac00daa08ba28..77084e53c75cd5f30fb4775103f80bcd662ab59f 100644 (file)
--- a/src/libsystemd-network/test-dhcp6-client.c
+++ b/src/libsystemd-network/test-dhcp6-client.c
@@ -217,14 +217,13 @@ static uint8_t fqdn_wire[16] = {
 static int test_advertise_option(sd_event *e) {
         _cleanup_(sd_dhcp6_lease_unrefp) sd_dhcp6_lease *lease = NULL;
         DHCP6Message *advertise = (DHCP6Message *)msg_advertise;
-        uint8_t *optval, *opt = msg_advertise + sizeof(DHCP6Message);
-        uint16_t optcode;
-        size_t optlen, len = sizeof(msg_advertise) - sizeof(DHCP6Message);
+        size_t len = sizeof(msg_advertise) - sizeof(DHCP6Message), pos = 0;
         be32_t val;
         uint8_t preference = 255;
         struct in6_addr addr;
         uint32_t lt_pref, lt_valid;
         int r;
+        uint8_t *opt;
         bool opt_clientid = false;
         struct in6_addr *addrs;
         char **domains;
@@ -232,14 +231,19 @@ static int test_advertise_option(sd_event *e) {
         if (verbose)
                 printf("* %s\n", __FUNCTION__);
 
+        assert_se(len >= sizeof(DHCP6Message));
+
         assert_se(dhcp6_lease_new(&lease) >= 0);
 
         assert_se(advertise->type == DHCP6_ADVERTISE);
         assert_se((be32toh(advertise->transaction_id) & 0x00ffffff) ==
                   0x0fb4e5);
 
-        while ((r = dhcp6_option_parse(&opt, &len, &optcode, &optlen,
-                                       &optval)) >= 0) {
+        while (pos < len) {
+                DHCP6Option *option = (DHCP6Option *)&advertise->options[pos];
+                const uint16_t optcode = be16toh(option->code);
+                const uint16_t optlen = be16toh(option->len);
+                uint8_t *optval = option->data;
 
                 switch(optcode) {
                 case SD_DHCP6_OPTION_CLIENTID:
@@ -261,9 +265,7 @@ static int test_advertise_option(sd_event *e) {
                         val = htobe32(120);
                         assert_se(!memcmp(optval + 8, &val, sizeof(val)));
 
-                        assert_se(dhcp6_option_parse_ia(&optval, &optlen,
-                                                        optcode,
-                                                        &lease->ia) >= 0);
+                        assert_se(dhcp6_option_parse_ia(option, &lease->ia) >= 0);
 
                         break;
 
@@ -309,11 +311,11 @@ static int test_advertise_option(sd_event *e) {
                 default:
                         break;
                 }
-        }
-
 
-        assert_se(r == -ENOMSG);
+                pos += sizeof(*option) + optlen;
+        }
 
+        assert_se(pos == len);
         assert_se(opt_clientid);
 
         sd_dhcp6_lease_reset_address_iter(lease);
@@ -415,15 +417,11 @@ static int test_client_send_reply(DHCP6Message *request) {
         return 0;
 }
 
-static int test_client_verify_request(DHCP6Message *request, uint8_t *option,
-                                      size_t len) {
+static int test_client_verify_request(DHCP6Message *request, size_t len) {
         _cleanup_(sd_dhcp6_lease_unrefp) sd_dhcp6_lease *lease = NULL;
-        uint8_t *optval;
-        uint16_t optcode;
-        size_t optlen;
+        size_t pos = 0;
         bool found_clientid = false, found_iana = false, found_serverid = false,
                 found_elapsed_time = false, found_fqdn = false;
-        int r;
         struct in6_addr addr;
         be32_t val;
         uint32_t lt_pref, lt_valid;
@@ -432,8 +430,14 @@ static int test_client_verify_request(DHCP6Message *request, uint8_t *option,
 
         assert_se(dhcp6_lease_new(&lease) >= 0);
 
-        while ((r = dhcp6_option_parse(&option, &len,
-                                       &optcode, &optlen, &optval)) >= 0) {
+        len -= sizeof(DHCP6Message);
+
+        while (pos < len) {
+                DHCP6Option *option = (DHCP6Option *)&request->options[pos];
+                uint16_t optcode = be16toh(option->code);
+                uint16_t optlen = be16toh(option->len);
+                uint8_t *optval = option->data;
+
                 switch(optcode) {
                 case SD_DHCP6_OPTION_CLIENTID:
                         assert_se(!found_clientid);
@@ -458,8 +462,7 @@ static int test_client_verify_request(DHCP6Message *request, uint8_t *option,
                         val = htobe32(120);
                         assert_se(!memcmp(optval + 8, &val, sizeof(val)));
 
-                        assert_se(!dhcp6_option_parse_ia(&optval, &optlen,
-                                                         optcode, &lease->ia));
+                        assert_se(!dhcp6_option_parse_ia(option, &lease->ia));
 
                         break;
 
@@ -489,9 +492,10 @@ static int test_client_verify_request(DHCP6Message *request, uint8_t *option,
                         assert_se(!memcmp(optval + 1, fqdn_wire, sizeof(fqdn_wire)));
                         break;
                 }
+
+                pos += sizeof(*option) + optlen;
         }
 
-        assert_se(r == -ENOMSG);
         assert_se(found_clientid && found_iana && found_serverid &&
                   found_elapsed_time);
 
@@ -526,19 +530,21 @@ static int test_client_send_advertise(DHCP6Message *solicit) {
         return 0;
 }
 
-static int test_client_verify_solicit(DHCP6Message *solicit, uint8_t *option,
-                                      size_t len) {
-        uint8_t *optval;
-        uint16_t optcode;
-        size_t optlen;
+static int test_client_verify_solicit(DHCP6Message *solicit, size_t len) {
         bool found_clientid = false, found_iana = false,
                 found_elapsed_time = false, found_fqdn = false;
-        int r;
+        size_t pos = 0;
 
         assert_se(solicit->type == DHCP6_SOLICIT);
 
-        while ((r = dhcp6_option_parse(&option, &len,
-                                       &optcode, &optlen, &optval)) >= 0) {
+        len -= sizeof(DHCP6Message);
+
+        while (pos < len) {
+                DHCP6Option *option = (DHCP6Option *)&solicit->options[pos];
+                uint16_t optcode = be16toh(option->code);
+                uint16_t optlen = be16toh(option->len);
+                uint8_t *optval = option->data;
+
                 switch(optcode) {
                 case SD_DHCP6_OPTION_CLIENTID:
                         assert_se(!found_clientid);
@@ -578,9 +584,11 @@ static int test_client_verify_solicit(DHCP6Message *solicit, uint8_t *option,
 
                         break;
                 }
+
+                pos += sizeof(*option) + optlen;
         }
 
-        assert_se(r == -ENOMSG);
+        assert_se(pos == len);
         assert_se(found_clientid && found_iana && found_elapsed_time);
 
         return 0;
@@ -623,17 +631,15 @@ static void test_client_information_cb(sd_dhcp6_client *client, int event,
         assert_se(sd_dhcp6_client_set_local_address(client, &address) >= 0);
 
         assert_se(sd_dhcp6_client_start(client) >= 0);
+
 }
 
 static int test_client_verify_information_request(DHCP6Message *information_request,
-                                                  uint8_t *option, size_t len) {
+                                                  size_t len) {
 
         _cleanup_(sd_dhcp6_lease_unrefp) sd_dhcp6_lease *lease = NULL;
-        uint8_t *optval;
-        uint16_t optcode;
-        size_t optlen;
+        size_t pos = 0;
         bool found_clientid = false, found_elapsed_time = false;
-        int r;
         struct in6_addr addr;
         uint32_t lt_pref, lt_valid;
 
@@ -641,8 +647,14 @@ static int test_client_verify_information_request(DHCP6Message *information_requ
 
         assert_se(dhcp6_lease_new(&lease) >= 0);
 
-        while ((r = dhcp6_option_parse(&option, &len,
-                                       &optcode, &optlen, &optval)) >= 0) {
+        len -= sizeof(DHCP6Message);
+
+        while (pos < len) {
+                DHCP6Option *option = (DHCP6Option *)&information_request->options[pos];
+                uint16_t optcode = be16toh(option->code);
+                uint16_t optlen = be16toh(option->len);
+                uint8_t *optval = option->data;
+
                 switch(optcode) {
                 case SD_DHCP6_OPTION_CLIENTID:
                         assert_se(!found_clientid);
@@ -671,9 +683,11 @@ static int test_client_verify_information_request(DHCP6Message *information_requ
 
                         break;
                 }
+
+                pos += sizeof(*option) + optlen;
         }
 
-        assert_se(r == -ENOMSG);
+        assert_se(pos == len);
         assert_se(found_clientid && found_elapsed_time);
 
         sd_dhcp6_lease_reset_address_iter(lease);
@@ -689,7 +703,6 @@ int dhcp6_network_send_udp_socket(int s, struct in6_addr *server_address,
         struct in6_addr mcast =
                 IN6ADDR_ALL_DHCP6_RELAY_AGENTS_AND_SERVERS_INIT;
         DHCP6Message *message;
-        uint8_t *option;
 
         assert_se(s == test_dhcp_fd[0]);
         assert_se(server_address);
@@ -699,21 +712,19 @@ int dhcp6_network_send_udp_socket(int s, struct in6_addr *server_address,
         assert_se(IN6_ARE_ADDR_EQUAL(server_address, &mcast));
 
         message = (DHCP6Message *)packet;
-        option = (uint8_t *)(message + 1);
-        len -= sizeof(DHCP6Message);
 
         assert_se(message->transaction_id & 0x00ffffff);
 
         if (test_client_message_num == 0) {
-                test_client_verify_information_request(message, option, len);
+                test_client_verify_information_request(message, len);
                 test_client_send_reply(message);
                 test_client_message_num++;
         } else if (test_client_message_num == 1) {
-                test_client_verify_solicit(message, option, len);
+                test_client_verify_solicit(message, len);
                 test_client_send_advertise(message);
                 test_client_message_num++;
         } else if (test_client_message_num == 2) {
-                test_client_verify_request(message, option, len);
+                test_client_verify_request(message, len);
                 test_client_send_reply(message);
                 test_client_message_num++;
         }