From e0a18b74a3b54bd9383b827139cea0df606e4378 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Oct 2018 12:11:29 +0200 Subject: [PATCH 1/5] dhcp6: constify things where we can --- src/libsystemd-network/dhcp6-internal.h | 4 ++-- src/libsystemd-network/dhcp6-option.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h index 63d8fe35f80..157fc0aadde 100644 --- a/src/libsystemd-network/dhcp6-internal.h +++ b/src/libsystemd-network/dhcp6-internal.h @@ -84,8 +84,8 @@ typedef struct DHCP6IA DHCP6IA; int dhcp6_option_append(uint8_t **buf, size_t *buflen, uint16_t code, size_t optlen, const void *optval); -int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, DHCP6IA *ia); -int dhcp6_option_append_pd(uint8_t *buf, size_t len, DHCP6IA *pd); +int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia); +int dhcp6_option_append_pd(uint8_t *buf, size_t len, const DHCP6IA *pd); 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); diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index d6c73b1f402..321d0d21dfa 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -79,7 +79,7 @@ int dhcp6_option_append(uint8_t **buf, size_t *buflen, uint16_t code, return 0; } -int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, DHCP6IA *ia) { +int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia) { uint16_t len; uint8_t *ia_hdr; size_t iaid_offset, ia_buflen, ia_addrlen = 0; @@ -165,7 +165,7 @@ int dhcp6_option_append_fqdn(uint8_t **buf, size_t *buflen, const char *fqdn) { return r; } -int dhcp6_option_append_pd(uint8_t *buf, size_t len, DHCP6IA *pd) { +int dhcp6_option_append_pd(uint8_t *buf, size_t len, const DHCP6IA *pd) { DHCP6Option *option = (DHCP6Option *)buf; size_t i = sizeof(*option) + sizeof(pd->ia_pd); DHCP6Address *prefix; From 3c290c03167cf9334cb419035587ff3057940eee Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Oct 2018 12:11:48 +0200 Subject: [PATCH 2/5] dhcp6: split assert_return() to be more debuggable when hit --- src/libsystemd-network/dhcp6-option.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index 321d0d21dfa..a4278528ab2 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -86,7 +86,10 @@ int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia) { DHCP6Address *addr; int r; - assert_return(buf && *buf && buflen && ia, -EINVAL); + assert_return(buf, -EINVAL); + assert_return(*buf, -EINVAL); + assert_return(buflen, -EINVAL); + assert_return(ia, -EINVAL); switch (ia->type) { case SD_DHCP6_OPTION_IA_NA: From 990668aa4cf04ea1c05791af97b1c05080378016 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Oct 2018 12:11:59 +0200 Subject: [PATCH 3/5] dhcp6: reduce whitespace a bit --- src/libsystemd-network/dhcp6-option.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index a4278528ab2..cbd4bc7a2a3 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -37,9 +37,9 @@ typedef struct DHCP6PDPrefixOption { uint8_t options[]; } _packed_ DHCP6PDPrefixOption; -#define DHCP6_OPTION_IA_NA_LEN (sizeof(struct ia_na)) -#define DHCP6_OPTION_IA_PD_LEN (sizeof(struct ia_pd)) -#define DHCP6_OPTION_IA_TA_LEN (sizeof(struct ia_ta)) +#define DHCP6_OPTION_IA_NA_LEN (sizeof(struct ia_na)) +#define DHCP6_OPTION_IA_PD_LEN (sizeof(struct ia_pd)) +#define DHCP6_OPTION_IA_TA_LEN (sizeof(struct ia_ta)) static int option_append_hdr(uint8_t **buf, size_t *buflen, uint16_t optcode, size_t optlen) { From 4dac5eaba4e419b29c97da38a8b1f82336c2c892 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Oct 2018 12:12:33 +0200 Subject: [PATCH 4/5] dhcp6: make sure we have enough space for the DHCP6 option header Fixes a vulnerability originally discovered by Felix Wilhelm from Google. CVE-2018-15688 LP: #1795921 https://bugzilla.redhat.com/show_bug.cgi?id=1639067 --- src/libsystemd-network/dhcp6-option.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index cbd4bc7a2a3..2806415100c 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -106,7 +106,7 @@ int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia) { return -EINVAL; } - if (*buflen < len) + if (*buflen < offsetof(DHCP6Option, data) + len) return -ENOBUFS; ia_hdr = *buf; From 20b55f853847378b85561a4e299604d27b5cd25b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Oct 2018 12:14:22 +0200 Subject: [PATCH 5/5] dhcp6: prefer offsetof() over sizeof() for structs with undefined sizes This doesn't change anything in the generated source, but I think makes semantically more sense, as these structures have undefined size, and we only want to know the size up to the data field in these cases. --- src/libsystemd-network/dhcp6-option.c | 30 +++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index 2806415100c..a2aac9a793d 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -49,14 +49,14 @@ static int option_append_hdr(uint8_t **buf, size_t *buflen, uint16_t optcode, assert_return(*buf, -EINVAL); assert_return(buflen, -EINVAL); - if (optlen > 0xffff || *buflen < optlen + sizeof(DHCP6Option)) + if (optlen > 0xffff || *buflen < optlen + offsetof(DHCP6Option, data)) return -ENOBUFS; option->code = htobe16(optcode); option->len = htobe16(optlen); - *buf += sizeof(DHCP6Option); - *buflen -= sizeof(DHCP6Option); + *buf += offsetof(DHCP6Option, data); + *buflen -= offsetof(DHCP6Option, data); return 0; } @@ -112,8 +112,8 @@ int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia) { ia_hdr = *buf; ia_buflen = *buflen; - *buf += sizeof(DHCP6Option); - *buflen -= sizeof(DHCP6Option); + *buf += offsetof(DHCP6Option, data); + *buflen -= offsetof(DHCP6Option, data); memcpy(*buf, (char*) ia + iaid_offset, len); @@ -131,7 +131,7 @@ int dhcp6_option_append_ia(uint8_t **buf, size_t *buflen, const DHCP6IA *ia) { *buf += sizeof(addr->iaaddr); *buflen -= sizeof(addr->iaaddr); - ia_addrlen += sizeof(DHCP6Option) + sizeof(addr->iaaddr); + ia_addrlen += offsetof(DHCP6Option, data) + sizeof(addr->iaaddr); } r = option_append_hdr(&ia_hdr, &ia_buflen, ia->type, len + ia_addrlen); @@ -213,7 +213,7 @@ static int option_parse_hdr(uint8_t **buf, size_t *buflen, uint16_t *optcode, si assert_return(optcode, -EINVAL); assert_return(optlen, -EINVAL); - if (*buflen < sizeof(DHCP6Option)) + if (*buflen < offsetof(DHCP6Option, data)) return -ENOMSG; len = be16toh(option->len); @@ -254,7 +254,7 @@ int dhcp6_option_parse_status(DHCP6Option *option, size_t len) { DHCP6StatusOption *statusopt = (DHCP6StatusOption *)option; if (len < sizeof(DHCP6StatusOption) || - be16toh(option->len) + sizeof(DHCP6Option) < sizeof(DHCP6StatusOption)) + be16toh(option->len) + offsetof(DHCP6Option, data) < sizeof(DHCP6StatusOption)) return -ENOBUFS; return be16toh(statusopt->status); @@ -267,7 +267,7 @@ static int dhcp6_option_parse_address(DHCP6Option *option, DHCP6IA *ia, uint32_t lt_valid, lt_pref; int r; - if (be16toh(option->len) + sizeof(DHCP6Option) < sizeof(*addr_option)) + if (be16toh(option->len) + offsetof(DHCP6Option, data) < sizeof(*addr_option)) return -ENOBUFS; lt_valid = be32toh(addr_option->iaaddr.lifetime_valid); @@ -280,8 +280,8 @@ static int dhcp6_option_parse_address(DHCP6Option *option, DHCP6IA *ia, return 0; } - if (be16toh(option->len) + sizeof(DHCP6Option) > sizeof(*addr_option)) { - r = dhcp6_option_parse_status((DHCP6Option *)addr_option->options, be16toh(option->len) + sizeof(DHCP6Option) - sizeof(*addr_option)); + if (be16toh(option->len) + offsetof(DHCP6Option, data) > sizeof(*addr_option)) { + r = dhcp6_option_parse_status((DHCP6Option *)addr_option->options, be16toh(option->len) + offsetof(DHCP6Option, data) - sizeof(*addr_option)); if (r != 0) return r < 0 ? r: 0; } @@ -307,7 +307,7 @@ static int dhcp6_option_parse_pdprefix(DHCP6Option *option, DHCP6IA *ia, uint32_t lt_valid, lt_pref; int r; - if (be16toh(option->len) + sizeof(DHCP6Option) < sizeof(*pdprefix_option)) + if (be16toh(option->len) + offsetof(DHCP6Option, data) < sizeof(*pdprefix_option)) return -ENOBUFS; lt_valid = be32toh(pdprefix_option->iapdprefix.lifetime_valid); @@ -320,8 +320,8 @@ static int dhcp6_option_parse_pdprefix(DHCP6Option *option, DHCP6IA *ia, return 0; } - if (be16toh(option->len) + sizeof(DHCP6Option) > sizeof(*pdprefix_option)) { - r = dhcp6_option_parse_status((DHCP6Option *)pdprefix_option->options, be16toh(option->len) + sizeof(DHCP6Option) - sizeof(*pdprefix_option)); + if (be16toh(option->len) + offsetof(DHCP6Option, data) > sizeof(*pdprefix_option)) { + r = dhcp6_option_parse_status((DHCP6Option *)pdprefix_option->options, be16toh(option->len) + offsetof(DHCP6Option, data) - sizeof(*pdprefix_option)); if (r != 0) return r < 0 ? r: 0; } @@ -453,7 +453,7 @@ int dhcp6_option_parse_ia(DHCP6Option *iaoption, DHCP6IA *ia) { case SD_DHCP6_OPTION_STATUS_CODE: - status = dhcp6_option_parse_status(option, optlen + sizeof(DHCP6Option)); + status = dhcp6_option_parse_status(option, optlen + offsetof(DHCP6Option, data)); if (status < 0) return status; if (status > 0) {