diff --git a/ndhc/README b/ndhc/README index 0990c60..8682251 100644 --- a/ndhc/README +++ b/ndhc/README @@ -52,3 +52,13 @@ go into an inactive state (until it is killed, or receives a SIGUSR1). You do not need to sleep between sending signals, as signals received are processed sequentially in the order they are received. +DHCP pitfalls +------------- + +Send a packet that has an options field set to: +DHCP-OPTION-OVERLOAD:3 + +Then in the file and sname fields: +DHCP-OPTION-OVERLOAD:3 + +I suspect some bad dhcp programs will hang given this input. diff --git a/ndhc/ifchange.c b/ndhc/ifchange.c index 985e0d9..22463e9 100644 --- a/ndhc/ifchange.c +++ b/ndhc/ifchange.c @@ -171,6 +171,7 @@ static void translate_option(int sockfd, struct dhcpMessage *packet, unsigned char *p; int i; struct dhcp_option *opt = NULL; + ssize_t optlen; if (!packet) return; @@ -187,7 +188,7 @@ static void translate_option(int sockfd, struct dhcpMessage *packet, memset(buf, '\0', sizeof(buf)); memset(buf2, '\0', sizeof(buf2)); - p = get_option(packet, code); + p = get_option(packet, code, &optlen); if (fill_options(buf2, p, opt, sizeof buf2 - 1) == -1) return; snprintf(buf, sizeof buf, "%s:", buf2); diff --git a/ndhc/options.c b/ndhc/options.c index e887da9..0194285 100644 --- a/ndhc/options.c +++ b/ndhc/options.c @@ -113,68 +113,83 @@ unsigned char *alloc_dhcp_client_id_option(unsigned char type, return alloc_option(DHCP_CLIENT_ID, data, sizeof data); } -/* Get an option with bounds checking (warning, result is not aligned) */ -uint8_t* get_option(struct dhcpMessage *packet, int code) +// Worker function for get_option(). +static uint8_t *do_get_option(uint8_t *buf, ssize_t buflen, int code, + char *overload, ssize_t *optlen) { - uint8_t *optionptr; - int len, rem, overload = 0; - enum { - OPTION_FIELD = 0, - FILE_FIELD = 1, - SNAME_FIELD = 2 - }; - enum { - FILE_FIELD101 = FILE_FIELD * 0x101, - SNAME_FIELD101 = SNAME_FIELD * 0x101, - }; - - /* option bytes: [code][len][data1][data2]..[dataLEN] */ - optionptr = packet->options; - rem = sizeof packet->options; - while (1) { - if (rem <= 0) { - log_warning("Bad packet, malformed option field."); - return NULL; - } - if (optionptr[OPT_CODE] == DHCP_PADDING) { - rem--; - optionptr++; + /* option bytes: [code][len]([data1][data2]..[dataLEN]) */ + *overload = 0; + while (buflen > 0) { + // Advance over padding. + if (buf[0] == DHCP_PADDING) { + buflen--; + buf++; continue; } - if (optionptr[OPT_CODE] == DHCP_END) { - if ((overload & FILE_FIELD101) == FILE_FIELD) { - /* can use packet->file, and didn't look at it yet */ - overload |= FILE_FIELD101; /* "we looked at it" */ - optionptr = packet->file; - rem = sizeof packet->file; - continue; - } - if ((overload & SNAME_FIELD101) == SNAME_FIELD) { - /* can use packet->sname, and didn't look at it yet */ - overload |= SNAME_FIELD101; /* "we looked at it" */ - optionptr = packet->sname; - rem = sizeof packet->sname; - continue; - } - break; + + // We hit the end. + if (buf[0] == DHCP_END) { + *optlen = 0; + return NULL; } - len = 2 + optionptr[OPT_LEN]; - rem -= len; - if (rem < 0) - continue; /* complain and return NULL */ - if (optionptr[OPT_CODE] == code) - return optionptr + OPT_DATA; + buflen -= buf[1] + 2; + if (buflen < 0) { + log_warning("Bad dhcp data: option length would exceed options field length"); + *optlen = 0; + return NULL; + } - if (optionptr[OPT_CODE] == DHCP_OPTION_OVERLOAD) { - overload |= optionptr[OPT_DATA]; + if (buf[0] == code) { + *optlen = buf[1]; + return buf + 2; + } + + if (buf[0] == DHCP_OPTION_OVERLOAD) { + if (buf[1] == 1) + *overload |= buf[2]; /* fall through */ } - optionptr += len; + buf += buf[1] + 2; } + log_warning("Bad dhcp data: unmarked end of options field"); + *optlen = 0; return NULL; } +/* Get an option with bounds checking (warning, result is not aligned) */ +uint8_t *get_option(struct dhcpMessage *packet, int code, ssize_t *optlen) +{ + uint8_t *option, *buf; + ssize_t buflen; + char overload, parsed_ff = 0; + + buf = packet->options; + buflen = sizeof packet->options; + + option = do_get_option(buf, buflen, code, &overload, optlen); + if (option) + return option; + + if (overload & 1) { + parsed_ff = 1; + option = do_get_option(packet->file, sizeof packet->file, + code, &overload, optlen); + if (option) + return option; + } + if (overload & 2) { + option = do_get_option(packet->sname, sizeof packet->sname, + code, &overload, optlen); + if (option) + return option; + if (!parsed_ff && overload & 1) + option = do_get_option(packet->file, sizeof packet->file, + code, &overload, optlen); + } + return option; +} + /* return the position of the 'end' option */ int end_option(uint8_t *optionptr) { diff --git a/ndhc/options.h b/ndhc/options.h index 4c5a22c..eeb1a40 100644 --- a/ndhc/options.h +++ b/ndhc/options.h @@ -79,7 +79,7 @@ unsigned char *alloc_option(unsigned char code, unsigned char *optdata, unsigned char *alloc_dhcp_client_id_option(unsigned char type, unsigned char *idstr, size_t idstrlen); -uint8_t *get_option(struct dhcpMessage *packet, int code); +uint8_t *get_option(struct dhcpMessage *packet, int code, ssize_t *optlen); int end_option(uint8_t *optionptr); int add_option_string(unsigned char *optionptr, unsigned char *string); int add_simple_option(unsigned char *optionptr, unsigned char code, uint32_t data); diff --git a/ndhc/packet.c b/ndhc/packet.c index 05053bb..29a5709 100644 --- a/ndhc/packet.c +++ b/ndhc/packet.c @@ -207,9 +207,10 @@ static void init_selecting_packet(struct client_state_t *cs, unsigned char *message) { unsigned char *temp = NULL; + ssize_t optlen; /* Must be a DHCPOFFER to one of our xid's */ if (*message == DHCPOFFER) { - if ((temp = get_option(packet, DHCP_SERVER_ID))) { + if ((temp = get_option(packet, DHCP_SERVER_ID, &optlen))) { /* Memcpy to a temp buffer to force alignment */ memcpy(&cs->serverAddr, temp, 4); cs->xid = packet->xid; @@ -230,8 +231,9 @@ static void dhcp_ack_or_nak_packet(struct client_state_t *cs, unsigned char *message) { unsigned char *temp = NULL; + ssize_t optlen; if (*message == DHCPACK) { - if (!(temp = get_option(packet, DHCP_LEASE_TIME))) { + if (!(temp = get_option(packet, DHCP_LEASE_TIME, &optlen))) { log_line("No lease time received, assuming 1h."); cs->lease = 60 * 60; } else { @@ -268,6 +270,7 @@ void handle_packet(struct client_state_t *cs) unsigned char *message = NULL; int len; struct dhcpMessage packet; + ssize_t optlen; if (cs->listenMode == LM_KERNEL) len = get_packet(&packet, cs->listenFd); @@ -290,7 +293,7 @@ void handle_packet(struct client_state_t *cs) return; } - if ((message = get_option(&packet, DHCP_MESSAGE_TYPE)) == NULL) { + if ((message = get_option(&packet, DHCP_MESSAGE_TYPE, &optlen)) == NULL) { log_line("couldnt get option from packet -- ignoring"); return; }