From b95d5a25777cc8910d2c2a921c68c778a3b30498 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Tue, 24 Mar 2026 22:07:49 +0000 Subject: [PATCH] Further tidying of DHCPv6 packet dissection code. Make things more logical, and put buffer overflow detection in one new function, opt6_first(). --- src/rfc3315.c | 79 ++++++++++++++------------------------------------- 1 file changed, 21 insertions(+), 58 deletions(-) diff --git a/src/rfc3315.c b/src/rfc3315.c index 9d67d5c..66d9192 100644 --- a/src/rfc3315.c +++ b/src/rfc3315.c @@ -39,9 +39,8 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu static void log6_opts(int nest, unsigned int xid, void *start_opts, void *end_opts); static void log6_packet(struct state *state, char *type, struct in6_addr *addr, char *string); static void log6_quiet(struct state *state, char *type, struct in6_addr *addr, char *string); -static void *opt6_find (uint8_t *opts, uint8_t *end, unsigned int search, unsigned int minsize); +static void *opt6_find (uint8_t *opts, uint8_t *end, unsigned int search, int minsize); static void *opt6_first(uint8_t *opt, uint8_t *end); -static void *opt6_next(uint8_t *opts, uint8_t *end); static unsigned int opt6_uint(unsigned char *opt, int offset, int size); static void get_context_tag(struct state *state, struct dhcp_context *context); static int check_ia(struct state *state, void *opt, void **endp, void **ia_option); @@ -63,6 +62,7 @@ static void calculate_times(struct dhcp_context *context, unsigned int *min_time #define opt6_len(opt) ((int)(opt6_uint(opt, -2, 2))) #define opt6_type(opt) (opt6_uint(opt, -4, 2)) #define opt6_ptr(opt, i) ((void *)&(((uint8_t *)(opt))[4+(i)])) +#define opt6_next(opt, end) (opt6_first(opt6_ptr((opt), opt6_len((opt))), (end))) #define opt6_user_vendor_ptr(opt, i) ((void *)&(((uint8_t *)(opt))[2+(i)])) #define opt6_user_vendor_len(opt) ((int)(opt6_uint(opt, -4, 2))) @@ -675,7 +675,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu for (c = state->context; c; c = c->current) c->flags &= ~CONTEXT_CONF_USED; - for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end)) + for (opt = opt6_first(state->packet_options, state->end); opt; opt = opt6_next(opt, state->end)) { void *ia_option, *ia_end; unsigned int min_time = 0xffffffff; @@ -839,7 +839,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu if (ignore) return 0; - for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end)) + for (opt = opt6_first(state->packet_options, state->end); opt; opt = opt6_next(opt, state->end)) { void *ia_option, *ia_end; unsigned int min_time = 0xffffffff; @@ -950,7 +950,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu log6_quiet(state, msg_type == DHCP6RENEW ? "DHCPRENEW" : "DHCPREBIND", NULL, NULL); - for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end)) + for (opt = opt6_first(state->packet_options, state->end); opt; opt = opt6_next(opt, state->end)) { void *ia_option, *ia_end; unsigned int min_time = 0xffffffff; @@ -1085,7 +1085,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu log6_quiet(state, "DHCPCONFIRM", NULL, NULL); - for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end)) + for (opt = opt6_first(state->packet_options, state->end); opt; opt = opt6_next(opt, state->end)) { void *ia_option, *ia_end; @@ -1161,7 +1161,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu log6_quiet(state, "DHCPRELEASE", NULL, NULL); - for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end)) + for (opt = opt6_first(state->packet_options, state->end); opt; opt = opt6_next(opt, state->end)) { void *ia_option, *ia_end; int made_ia = 0; @@ -1226,7 +1226,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu log6_quiet(state, "DHCPDECLINE", NULL, NULL); - for (opt = state->packet_options; opt; opt = opt6_next(opt, state->end)) + for (opt = opt6_first(state->packet_options, state->end); opt; opt = opt6_next(opt, state->end)) { void *ia_option, *ia_end; int made_ia = 0; @@ -1598,10 +1598,7 @@ static int check_ia(struct state *state, void *opt, void **endp, void **ia_optio { *ia_option = NULL; - /* must be a minimal option to check without stepping outside received packet. */ - if (opt6_ptr(opt, 4) > state->end) - return 0; - + /* callee ensures packet is long enough for opt6_len(opt) to be valid and believe-able. */ state->ia_type = opt6_type(opt); if (state->ia_type != OPTION6_IA_NA && state->ia_type != OPTION6_IA_TA) @@ -1613,10 +1610,7 @@ static int check_ia(struct state *state, void *opt, void **endp, void **ia_optio if (state->ia_type == OPTION6_IA_TA && opt6_len(opt) < 4) return 0; - /* Check we don't overflow the received packet. */ - if ((*endp = opt6_ptr(opt, opt6_len(opt))) > state->end) - return 0; - + *endp = opt6_ptr(opt, opt6_len(opt)); state->iaid = opt6_uint(opt, 0, 4); *ia_option = opt6_find(opt6_ptr(opt, state->ia_type == OPTION6_IA_NA ? 12 : 4), *endp, OPTION6_IAADDR, 24); @@ -2007,10 +2001,10 @@ static void log6_opts(int nest, unsigned int xid, void *start_opts, void *end_op void *opt; char *desc = nest ? "nest" : "sent"; - if (!option_bool(OPT_LOG_OPTS) || start_opts == end_opts) + if (!option_bool(OPT_LOG_OPTS)) return; - for (opt = start_opts; opt; opt = opt6_next(opt, end_opts)) + for (opt = opt6_first(start_opts, end_opts); opt; opt = opt6_next(opt, end_opts)) { int type = opt6_type(opt); void *ia_options = NULL; @@ -2104,35 +2098,20 @@ static void log6_packet(struct state *state, char *type, struct in6_addr *addr, string ? string : ""); } -static void *opt6_find (uint8_t *opts, uint8_t *end, unsigned int search, unsigned int minsize) +static void *opt6_find(uint8_t *opts, uint8_t *end, unsigned int search, int minsize) { - u16 opt, opt_len; - void *start; - - if (!opts) - return NULL; - - while (1) - { - if (end - opts < 4) - return NULL; - - start = opts; - GETSHORT(opt, opts); - GETSHORT(opt_len, opts); - - if (opt_len > (end - opts)) - return NULL; - - if (opt == search && (opt_len >= minsize)) - return start; - - opts += opt_len; - } + for (opts = opt6_first(opts, end); opts; opts = opt6_next(opts, end)) + if (opt6_type(opts) == search && opt6_len(opts) >= minsize) + return opts; + + return NULL; } static void *opt6_first(uint8_t *opt, uint8_t *end) { + if (!opt) + return NULL; + /* make sure we have option number and length. */ if ((uint8_t *)opt6_ptr(opt, 0) > end) return NULL; @@ -2144,22 +2123,6 @@ static void *opt6_first(uint8_t *opt, uint8_t *end) return opt; } -static void *opt6_next(uint8_t *opts, uint8_t *end) -{ - u16 opt_len; - - if (end - opts < 4) - return NULL; - - opts += 2; - GETSHORT(opt_len, opts); - - if (opt_len >= (end - opts)) - return NULL; - - return opts + opt_len; -} - static unsigned int opt6_uint(unsigned char *opt, int offset, int size) { /* this worries about unaligned data and byte order */