Enforce stronger type checking on DHCP option values. Fix some endianness

issues as well.

ndhc/dhcp.c

@@ -569,7 +569,7 @@ static struct dhcpmsg init_packet(char type, uint32_t xid)
         .options[0] = DHCP_END,
         .xid = xid,
     };
-    add_u32_option(&packet, DHCP_MESSAGE_TYPE, type);
+    add_u8_option(&packet, DHCP_MESSAGE_TYPE, type);
     memcpy(packet.chaddr, client_config.arp, 6);
     add_option_clientid(&packet);
     add_option_hostname(&packet);
@@ -581,7 +581,7 @@ int send_discover(struct client_state_t *cs)
     struct dhcpmsg packet = init_packet(DHCPDISCOVER, cs->xid);
     if (cs->clientAddr)
         add_u32_option(&packet, DHCP_REQUESTED_IP, cs->clientAddr);
-    add_u32_option(&packet, DHCP_MAX_SIZE,
+    add_u16_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
     add_option_vendor(&packet);
@@ -594,7 +594,7 @@ int send_selecting(struct client_state_t *cs)
     struct dhcpmsg packet = init_packet(DHCPREQUEST, cs->xid);
     add_u32_option(&packet, DHCP_REQUESTED_IP, cs->clientAddr);
     add_u32_option(&packet, DHCP_SERVER_ID, cs->serverAddr);
-    add_u32_option(&packet, DHCP_MAX_SIZE,
+    add_u16_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
     add_option_vendor(&packet);
@@ -607,7 +607,7 @@ int send_renew(struct client_state_t *cs)
 {
     struct dhcpmsg packet = init_packet(DHCPREQUEST, cs->xid);
     packet.ciaddr = cs->clientAddr;
-    add_u32_option(&packet, DHCP_MAX_SIZE,
+    add_u16_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
     add_option_vendor(&packet);
@@ -620,7 +620,7 @@ int send_rebind(struct client_state_t *cs)
     struct dhcpmsg packet = init_packet(DHCPREQUEST, cs->xid);
     packet.ciaddr = cs->clientAddr;
     add_u32_option(&packet, DHCP_REQUESTED_IP, cs->clientAddr);
-    add_u32_option(&packet, DHCP_MAX_SIZE,
+    add_u16_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
     add_option_vendor(&packet);

ndhc/dhcp.h

@@ -60,8 +60,8 @@ struct dhcpmsg {
     uint8_t chaddr[16];  // Client MAC address
     uint8_t sname[64];   // Server host name (optional); null-terminated string
     uint8_t file[128];   // boot file name, null-terminated string
-    uint32_t cookie;
+    uint32_t cookie;     // Magic number cookie that starts DHCP options
-    uint8_t options[308]; /* 312 - cookie */
+    uint8_t options[308]; // Size of options excluding the cookie.
 };
 
 struct ip_udp_dhcp_packet {

ndhc/options.c

@@ -227,7 +227,7 @@ size_t add_option_string(struct dhcpmsg *packet, uint8_t code, char *str,
         return 0;
     }
 
-    size_t end = get_end_option_idx(packet);
+    ssize_t end = get_end_option_idx(packet);
     if (end == -1) {
         log_warning("add_option_string: Buffer has no DHCP_END marker");
         return 0;
@@ -243,30 +243,70 @@ size_t add_option_string(struct dhcpmsg *packet, uint8_t code, char *str,
     return len;
 }
 
-// XXX: length=1 and length=2 will fail if data is big-endian.
+static ssize_t add_option_check(struct dhcpmsg *packet, uint8_t code,
-size_t add_u32_option(struct dhcpmsg *packet, uint8_t code, uint32_t data)
+                                uint8_t rlen)
 {
     size_t length = option_length(code);
-
+    if (length != rlen) {
-    if (!length) {
+        log_warning("add_u%01u_option: length mismatch code=0x%02x len=%01u",
-        log_warning("add_u32_option: option code 0x%02x has 0 length", code);
+                    rlen*8, code, length);
-        return 0;
+        return -1;
     }
-    size_t end = get_end_option_idx(packet);
+    ssize_t end = get_end_option_idx(packet);
     if (end == -1) {
-        log_warning("add_u32_option: Buffer has no DHCP_END marker");
+        log_warning("add_u%01u_option: Buffer has no DHCP_END marker", rlen*8);
-        return 0;
+        return -1;
     }
-    if (end + 2 + length >= sizeof packet->options) {
+    if (end + 2 + rlen >= sizeof packet->options) {
-        log_warning("add_u32_option: No space for option 0x%02x", code);
+        log_warning("add_u%01u_option: No space for option 0x%02x",
-        return 0;
+                    rlen*8, code);
+        return -1;
     }
+    return end;
+}
+
+size_t add_u8_option(struct dhcpmsg *packet, uint8_t code, uint8_t data)
+{
+    ssize_t end = add_option_check(packet, code, 1);
+    if (end < 0)
+        return 0;
     packet->options[end] = code;
-    packet->options[end+1] = length;
+    packet->options[end+1] = 1;
-    // XXX: this is broken: it's alignment-safe, but the endianness is wrong
+    packet->options[end+2] = data;
-    memcpy(packet->options + end + 2, &data, length);
+    packet->options[end+3] = DHCP_END;
-    packet->options[end+length+2] = DHCP_END;
+    return 3;
-    return length+2;
+}
+
+// Data should be in network byte order.
+size_t add_u16_option(struct dhcpmsg *packet, uint8_t code, uint16_t data)
+{
+    ssize_t end = add_option_check(packet, code, 2);
+    if (end < 0)
+        return 0;
+    uint8_t *dp = (uint8_t *)&data;
+    packet->options[end] = code;
+    packet->options[end+1] = 2;
+    packet->options[end+2] = dp[0];
+    packet->options[end+3] = dp[1];
+    packet->options[end+4] = DHCP_END;
+    return 4;
+}
+
+// Data should be in network byte order.
+size_t add_u32_option(struct dhcpmsg *packet, uint8_t code, uint32_t data)
+{
+    ssize_t end = add_option_check(packet, code, 4);
+    if (end < 0)
+        return 0;
+    uint8_t *dp = (uint8_t *)&data;
+    packet->options[end] = code;
+    packet->options[end+1] = 4;
+    packet->options[end+2] = dp[0];
+    packet->options[end+3] = dp[1];
+    packet->options[end+4] = dp[2];
+    packet->options[end+5] = dp[3];
+    packet->options[end+6] = DHCP_END;
+    return 6;
 }
 
 // Add a paramater request list for stubborn DHCP servers

ndhc/options.h

@@ -87,6 +87,8 @@ uint8_t *get_option_data(struct dhcpmsg *packet, int code, ssize_t *optlen);
 ssize_t get_end_option_idx(struct dhcpmsg *packet);
 size_t add_option_string(struct dhcpmsg *packet, uint8_t code, char *str,
                          size_t slen);
+size_t add_u8_option(struct dhcpmsg *packet, uint8_t code, uint8_t data);
+size_t add_u16_option(struct dhcpmsg *packet, uint8_t code, uint16_t data);
 size_t add_u32_option(struct dhcpmsg *packet, uint8_t code, uint32_t data);
 size_t add_option_request_list(struct dhcpmsg *packet);