Don't use malloc in ndhc. The only place it was used is in the options code.

Allow the user to specify the vendor identification option value using the
-V switch.  The default value is still "ndhc".

ndhc/config.h

@@ -40,15 +40,16 @@ struct client_state_t {
 };
 
 struct client_config_t {
-    char foreground;             /* Do not fork */
+    char foreground;             // Do not fork
-    char quit_after_lease;       /* Quit after obtaining lease */
+    char quit_after_lease;       // Quit after obtaining lease
-    char abort_if_no_lease;      /* Abort if no lease */
+    char abort_if_no_lease;      // Abort if no lease
-    char background_if_no_lease;    /* Fork to background if no lease */
+    char background_if_no_lease; // Fork to background if no lease
-    char *interface;             /* The name of the interface to use */
+    char *interface;             // The name of the interface to use
-    uint8_t *clientid;           /* Optional client id to use (unterminated) */
+    char clientid[64];           // Optional client id to use
-    uint8_t *hostname;           /* Optional hostname to use (unterminated) */
+    char hostname[64];           // Optional hostname to use
-    int ifindex;                 /* Index number of the interface to use */
+    char vendor[64];             // Vendor identification that will be sent
-    uint8_t arp[6];              /* Our arp address */
+    int ifindex;                 // Index number of the interface to use
+    uint8_t arp[6];              // Our arp address
 };
 
 extern struct client_config_t client_config;

ndhc/ndhc.c

@@ -54,7 +54,6 @@
 #include "cap.h"
 #include "strl.h"
 #include "pidfile.h"
-#include "malloc.h"
 #include "io.h"
 
 #define VERSION "1.0"
@@ -177,7 +176,7 @@ static void do_work(void)
 int main(int argc, char **argv)
 {
     char chroot_dir[MAX_PATH_LENGTH] = "";
-    int c, len;
+    int c;
     struct passwd *pwd;
     uid_t uid = 0;
     gid_t gid = 0;
@@ -188,32 +187,30 @@ int main(int argc, char **argv)
         {"pidfile",     required_argument,  0, 'p'},
         {"leasefile",   required_argument,  0, 'l'},
         {"hostname",    required_argument,  0, 'H'},
-        {"hostname",    required_argument,      0, 'h'},
+        {"hostname",    required_argument,  0, 'h'},
         {"interface",   required_argument,  0, 'i'},
         {"now",         no_argument,        0, 'n'},
-        {"quit",    no_argument,        0, 'q'},
+        {"quit",        no_argument,        0, 'q'},
-        {"request", required_argument,  0, 'r'},
+        {"request",     required_argument,  0, 'r'},
-        {"version", no_argument,        0, 'v'},
+        {"version",     no_argument,        0, 'v'},
-        {"user",        required_argument,      0, 'u'},
+        {"vendorid",    required_argument,  0, 'V'},
-        {"chroot",      required_argument,      0, 'C'},
+        {"user",        required_argument,  0, 'u'},
-        {"help",    no_argument,        0, '?'},
+        {"chroot",      required_argument,  0, 'C'},
+        {"help",        no_argument,        0, '?'},
         {0, 0, 0, 0}
     };
 
     /* get options */
     while (1) {
         int option_index = 0;
-        c = getopt_long(argc, argv, "c:fbp:H:h:i:np:l:qr:u:C:v", arg_options,
+        c = getopt_long(argc, argv, "c:fbp:H:h:i:np:l:qr:u:C:vV:", arg_options,
                         &option_index);
         if (c == -1) break;
 
         switch (c) {
             case 'c':
-                len = strlen(optarg) > 64 ? 64 : strlen(optarg);
+                strlcpy(client_config.clientid, optarg,
-                if (client_config.clientid)
+                        sizeof client_config.clientid);
-                    free(client_config.clientid);
-                client_config.clientid =
-                    alloc_dhcp_client_id_option(0, (uint8_t *)optarg, len);
                 break;
             case 'f':
                 client_config.foreground = 1;
@@ -231,11 +228,8 @@ int main(int argc, char **argv)
                 break;
             case 'h':
             case 'H':
-                len = strlen(optarg) > 64 ? 64 : strlen(optarg);
+                strlcpy(client_config.hostname, optarg,
-                if (client_config.hostname)
+                        sizeof client_config.hostname);
-                    free(client_config.hostname);
-                client_config.hostname =
-                    alloc_option(DHCP_HOST_NAME, (uint8_t *)optarg, len);
                 break;
             case 'i':
                 client_config.interface = optarg;
@@ -266,6 +260,10 @@ int main(int argc, char **argv)
                 printf("ndhc, version " VERSION "\n\n");
                 exit(EXIT_SUCCESS);
                 break;
+            case 'V':
+                strlcpy(client_config.vendor, optarg,
+                        sizeof client_config.vendor);
+                break;
             default:
                 show_usage();
         }
@@ -290,11 +288,6 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
-    if (!client_config.clientid) {
-        client_config.clientid =
-            alloc_dhcp_client_id_option(1, client_config.arp, 6);
-    }
-
     open_leasefile();
 
     if (chdir(chroot_dir)) {

ndhc/options.c

@@ -24,7 +24,6 @@
 
 #include "options.h"
 #include "log.h"
-#include "malloc.h"
 
 struct dhcp_option {
     char name[10];
@@ -123,35 +122,6 @@ static size_t sizeof_option(uint8_t code, size_t datalen)
     return 2 + datalen;
 }
 
-uint8_t *alloc_option(uint8_t code, uint8_t *optdata, size_t datalen)
-{
-    uint8_t *buf;
-    size_t len = sizeof_option(code, datalen);
-    buf = xmalloc(len);
-    if (!optdata)
-        datalen = 0;
-    if ((code == DHCP_PADDING || code == DHCP_END) && len >= 1) {
-        buf[0] = code;
-    } else if (datalen <= 255 && len >= 2 + datalen) {
-        buf[0] = code;
-        buf[1] = datalen;
-        memcpy(buf + 2, optdata, datalen);
-    }
-    return buf;
-}
-
-// This is tricky -- the data must be prefixed by one byte indicating the
-// type of ARP MAC address (1 for ethernet) or 0 for a purely symbolic
-// identifier.
-uint8_t *alloc_dhcp_client_id_option(uint8_t type, uint8_t *idstr,
-                                     size_t idstrlen)
-{
-    uint8_t data[idstrlen + 1];
-    data[0] = type;
-    memcpy(data + 1, idstr, idstrlen);
-    return alloc_option(DHCP_CLIENT_ID, data, sizeof data);
-}
-
 // Worker function for get_option_data().  Optlen will be set to the length
 // of the option data.
 static uint8_t *do_get_option_data(uint8_t *buf, ssize_t buflen, int code,
@@ -248,74 +218,66 @@ ssize_t get_end_option_idx(struct dhcpmsg *packet)
 
 // add an option string to the options (an option string contains an option
 // code, length, then data)
-size_t add_option_string(struct dhcpmsg *packet, uint8_t *optstr)
+size_t add_option_string(struct dhcpmsg *packet, uint8_t code, char *str,
+                         size_t slen)
 {
-    size_t end = get_end_option_idx(packet);
+    size_t len = sizeof_option(code, slen);
-    size_t datalen = optstr[1];
+    if (slen > 255 || len != slen + 2) {
+        log_warning("add_option_string: Length checks failed.");
+        return 0;
+    }
 
+    size_t end = get_end_option_idx(packet);
     if (end == -1) {
         log_warning("add_option_string: Buffer has no DHCP_END marker");
         return 0;
     }
-    // end position + optstr length + option code/length + end option
+    if (end + len >= sizeof packet->options) {
-    if (end + datalen + 2 + 1 >= sizeof packet->options) {
+        log_warning("add_option_string: No space for option 0x%02x", code);
-        log_warning("add_option_string: No space for option 0x%02x", optstr[0]);
         return 0;
     }
-    memcpy(packet->options + end, optstr, datalen + 2);
+    packet->options[end] = code;
-    packet->options[end + datalen + 2] = DHCP_END;
+    packet->options[end+1] = slen;
-    return datalen + 2;
+    memcpy(packet->options + end + 2, str, slen);
+    packet->options[end+len] = DHCP_END;
+    return len;
 }
 
+// XXX: length=1 and length=2 will fail if data is big-endian.
 size_t add_u32_option(struct dhcpmsg *packet, uint8_t code, uint32_t data)
 {
-    int length = 0;
+    size_t length = option_length(code);
-    uint8_t option[6];
-
-    length = option_length(code);
-
-    option[0] = code;
-    option[1] = length;
 
     if (!length) {
         log_warning("add_u32_option: option code 0x%02x has 0 length", code);
         return 0;
     }
-
+    size_t end = get_end_option_idx(packet);
-    if (length == 1) {
+    if (end == -1) {
-        uint8_t t = (uint8_t)data;
+        log_warning("add_u32_option: Buffer has no DHCP_END marker");
-        memcpy(option + 2, &t, 1);
+        return 0;
-    } else if (length == 2) {
-        uint16_t t = (uint16_t)data;
-        memcpy(option + 2, &t, 2);
-    } else if (length == 4) {
-        uint32_t t = (uint32_t)data;
-        memcpy(option + 2, &t, 4);
     }
-    return add_option_string(packet, option);
+    if (end + 2 + length >= sizeof packet->options) {
+        log_warning("add_u32_option: No space for option 0x%02x", code);
+        return 0;
+    }
+    packet->options[end] = code;
+    packet->options[end+1] = length;
+    // XXX: this is broken: it's alignment-safe, but the endianness is wrong
+    memcpy(packet->options + end + 2, &data, length);
+    packet->options[end+length+2] = DHCP_END;
+    return length+2;
 }
 
 // Add a paramater request list for stubborn DHCP servers
-void add_option_request_list(struct dhcpmsg *packet)
+size_t add_option_request_list(struct dhcpmsg *packet)
 {
     uint8_t reqdata[256];
-    reqdata[0] = DHCP_PARAM_REQ;
+    size_t j = 0;
-    int j = 2;
     for (int i = 0; options[i].code; i++) {
         if (options[i].type & OPTION_REQ)
             reqdata[j++] = options[i].code;
     }
-    reqdata[1] = j - 2;
+    return add_option_string(packet, DHCP_PARAM_REQ, (char *)reqdata, j);
-    add_option_string(packet, reqdata);
-}
-
-void add_option_vendor_string(struct dhcpmsg *packet)
-{
-    struct vendor  {
-        char vendor;
-        char length;
-        char str[sizeof "ndhc"];
-    } vendor_id = { DHCP_VENDOR,  sizeof "ndhc" - 1, "ndhc"};
-    add_option_string(packet, (uint8_t *)&vendor_id);
 }
 

ndhc/options.h

@@ -82,16 +82,12 @@ const char *option_name(uint8_t code);
 enum option_type option_type(uint8_t code);
 uint8_t option_length(uint8_t code);
 int option_valid_list(uint8_t code);
-uint8_t *alloc_option(uint8_t code, uint8_t *optdata, size_t datalen);
-
-uint8_t *alloc_dhcp_client_id_option(uint8_t type, uint8_t *idstr,
-                                     size_t idstrlen);
 
 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 *optstr);
+size_t add_option_string(struct dhcpmsg *packet, uint8_t code, char *str,
+                         size_t slen);
 size_t add_u32_option(struct dhcpmsg *packet, uint8_t code, uint32_t data);
-void add_option_request_list(struct dhcpmsg *packet);
+size_t add_option_request_list(struct dhcpmsg *packet);
-void add_option_vendor_string(struct dhcpmsg *packet);
 
 #endif

ndhc/packet.c

@@ -533,6 +533,31 @@ void handle_packet(struct client_state_t *cs)
     packet_action(cs, &packet, message);
 }
 
+static void add_option_vendor(struct dhcpmsg *packet)
+{
+    size_t len = strlen(client_config.vendor);
+    if (len)
+        add_option_string(packet, DHCP_VENDOR, client_config.vendor, len);
+    else
+        add_option_string(packet, DHCP_VENDOR, "ndhc", sizeof "ndhc" - 1);
+}
+
+static void add_option_clientid(struct dhcpmsg *packet)
+{
+    size_t len = strlen(client_config.clientid);
+    if (len)
+        add_option_string(packet, DHCP_CLIENT_ID, client_config.clientid, len);
+    else
+        add_option_string(packet, DHCP_CLIENT_ID, (char *)client_config.arp, 6);
+}
+
+static void add_option_hostname(struct dhcpmsg *packet)
+{
+    size_t len = strlen(client_config.hostname);
+    if (len)
+        add_option_string(packet, DHCP_HOST_NAME, client_config.hostname, len);
+}
+
 // Initialize a DHCP client packet that will be sent to a server
 static struct dhcpmsg init_packet(char type, uint32_t xid)
 {
@@ -546,9 +571,8 @@ static struct dhcpmsg init_packet(char type, uint32_t xid)
     };
     add_u32_option(&packet, DHCP_MESSAGE_TYPE, type);
     memcpy(packet.chaddr, client_config.arp, 6);
-    add_option_string(&packet, client_config.clientid);
+    add_option_clientid(&packet);
-    if (client_config.hostname)
+    add_option_hostname(&packet);
-        add_option_string(&packet, client_config.hostname);
     return packet;
 }
 
@@ -560,7 +584,7 @@ int send_discover(struct client_state_t *cs)
     add_u32_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
-    add_option_vendor_string(&packet);
+    add_option_vendor(&packet);
     log_line("Sending discover...");
     return send_dhcp_raw(&packet);
 }
@@ -573,7 +597,7 @@ int send_selecting(struct client_state_t *cs)
     add_u32_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
-    add_option_vendor_string(&packet);
+    add_option_vendor(&packet);
     log_line("Sending select for %s...",
              inet_ntoa((struct in_addr){.s_addr = cs->clientAddr}));
     return send_dhcp_raw(&packet);
@@ -586,7 +610,7 @@ int send_renew(struct client_state_t *cs)
     add_u32_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
-    add_option_vendor_string(&packet);
+    add_option_vendor(&packet);
     log_line("Sending renew...");
     return send_dhcp_cooked(&packet, cs->clientAddr, cs->serverAddr);
 }
@@ -599,7 +623,7 @@ int send_rebind(struct client_state_t *cs)
     add_u32_option(&packet, DHCP_MAX_SIZE,
                    htons(sizeof(struct ip_udp_dhcp_packet)));
     add_option_request_list(&packet);
-    add_option_vendor_string(&packet);
+    add_option_vendor(&packet);
     log_line("Sending rebind...");
     return send_dhcp_raw(&packet);
 }