Convert the remaining strnk* calls to use snprintf instead, and make sure

to detect truncations and failures in all cases.

ndhc/duiaid.c

@@ -36,7 +36,6 @@
 #include <limits.h>
 #include <errno.h>
 #include "duiaid.h"
-#include "strl.h"
 #include "log.h"
 #include "random.h"
 #include "io.h"

ndhc/ifchd-parse.rl

@@ -27,14 +27,16 @@
  */
 
 #include <stddef.h>
+#include <stdbool.h>
+#include <stdio.h>
 #include <string.h>
 #include <arpa/inet.h>
 
 #include "ifchd-parse.h"
 #include "ifchd.h"
 #include "log.h"
-#include "strl.h"
 #include "ifset.h"
+#include "ndhc.h"
 
 %%{
     machine ipv4set_parser;
@@ -183,11 +185,15 @@ int execute_buffer(char *newbuf)
     char buf[MAX_BUF * 2];
     char tb[MAX_BUF];
 
-    if (strnkcpy(buf, cl.ibuf, sizeof buf))
+    ssize_t buflen = snprintf(buf, sizeof buf, "%s%s", cl.ibuf, newbuf);
-        goto buftooshort;
+    if (buflen < 0) {
-    if (strnkcat(buf, newbuf, sizeof buf)) {
+        log_error("%s: (%s) snprintf1 failed; your system is broken?",
-buftooshort:
+                  client_config.interface, __func__);
-        log_line("error: input is too long for buffer");
+        return -1;
+    }
+    if ((size_t)buflen >= sizeof buf) {
+        log_error("%s: (%s) input is too long for buffer",
+                  client_config.interface, __func__);
         return -1;
     }
 
@@ -204,14 +210,26 @@ buftooshort:
     size_t bytes_left = pe - p;
     if (bytes_left > 0) {
         size_t taken = init_siz - bytes_left;
-        strnkcpy(cl.ibuf, buf + taken, MAX_BUF);
+        ssize_t ilen = snprintf(cl.ibuf, sizeof cl.ibuf, "%s", buf + taken);
+        if (ilen < 0) {
+            log_error("%s: (%s) snprintf2 failed; your system is broken?",
+                      client_config.interface, __func__);
+            return -1;
+        }
+        if ((size_t)ilen >= sizeof buf) {
+            log_error("%s: (%s) unconsumed input too long for buffer",
+                      client_config.interface, __func__);
+            return -1;
+        }
     }
 
     if (cs < ifchd_parser_first_final) {
-        log_line("error: received invalid commands");
+        log_error("%s: ifch received invalid commands",
+                  client_config.interface);
         return -1;
     }
-    log_line("Commands received and successfully executed.");
+    log_line("%s: Commands received and successfully executed.",
+             client_config.interface);
     return 0;
 }
 

ndhc/ifset.c

@@ -27,6 +27,7 @@
  */
 
 #include <unistd.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -51,7 +52,6 @@
 #include "ifchd.h"
 #include "ndhc.h"
 #include "log.h"
-#include "strl.h"
 #include "nl.h"
 
 static uint32_t ifset_nl_seq = 1;

ndhc/leasefile.c

@@ -40,7 +40,6 @@
 #include "leasefile.h"
 #include "ndhc.h"
 #include "log.h"
-#include "strl.h"
 #include "io.h"
 #include "defines.h"
 
@@ -76,12 +75,21 @@ void open_leasefile(void)
 
 void write_leasefile(struct in_addr ipnum)
 {
-    char ip[INET_ADDRSTRLEN+2];
+    char ip[INET_ADDRSTRLEN];
+    char out[INET_ADDRSTRLEN*2];
     int ret;
-    if (leasefilefd < 0)
+    if (leasefilefd < 0) {
+        log_error("%s: (%s) leasefile fd < 0; no leasefile will be written",
+                  client_config.interface, __func__);
         return;
+    }
     inet_ntop(AF_INET, &ipnum, ip, sizeof ip);
-    strnkcat(ip, "\n", sizeof ip);
+    ssize_t olen = snprintf(out, sizeof out, "%s\n", ip);
+    if (olen < 0 || (size_t)olen >= sizeof ip) {
+        log_error("%s: (%s) snprintf failed; return=%d",
+                  client_config.interface, __func__, olen);
+        return;
+    }
   retry_trunc:
     ret = ftruncate(leasefilefd, 0);
     switch (ret) {
@@ -94,7 +102,7 @@ void write_leasefile(struct in_addr ipnum)
             return;
     }
     lseek(leasefilefd, 0, SEEK_SET);
-    ret = safe_write(leasefilefd, ip, strlen(ip));
+    ret = safe_write(leasefilefd, out, strlen(out));
     if (ret == -1)
         log_warning("%s: Failed to write ip to lease file.",
                     client_config.interface);

ndhc/ndhc.c

@@ -65,7 +65,6 @@
 #include "log.h"
 #include "chroot.h"
 #include "cap.h"
-#include "strl.h"
 #include "pidfile.h"
 #include "io.h"
 #include "seccomp.h"
@@ -419,6 +418,19 @@ void background(void)
         write_pid(pidfile);
 }
 
+static void copy_cmdarg(char *dest, char *src, size_t destlen, char *argname)
+{
+    ssize_t olen = snprintf(dest, destlen, "%s", src);
+    if (olen < 0) {
+        log_error("snprintf failed on %s; your system is broken?", argname);
+        exit(EXIT_FAILURE);
+    }
+    if ((size_t)olen >= destlen) {
+        log_error("snprintf would truncate %s arg; it's too long", argname);
+        exit(EXIT_FAILURE);
+    }
+}
+
 int main(int argc, char **argv)
 {
     static const struct option arg_options[] = {
@@ -470,18 +482,19 @@ int main(int argc, char **argv)
                 gflags_detach = 1;
                 break;
             case 'p':
-                strnkcpy(pidfile, optarg, sizeof pidfile);
+                copy_cmdarg(pidfile, optarg, sizeof pidfile, "pidfile");
                 break;
             case 'P':
-                strnkcpy(pidfile_ifch, optarg, sizeof pidfile_ifch);
+                copy_cmdarg(pidfile_ifch, optarg, sizeof pidfile_ifch,
+                            "ifch-pidfile");
                 break;
             case 'h':
-                strnkcpy(client_config.hostname, optarg,
+                copy_cmdarg(client_config.hostname, optarg,
-                         sizeof client_config.hostname);
+                            sizeof client_config.hostname, "hostname");
                 break;
             case 'i':
-                strnkcpy(client_config.interface, optarg,
+                copy_cmdarg(client_config.interface, optarg,
-                         sizeof client_config.interface);
+                            sizeof client_config.interface, "interface");
                 break;
             case 'n':
                 client_config.abort_if_no_lease = 1;
@@ -527,10 +540,10 @@ int main(int argc, char **argv)
                 break;
             }
             case 'C':
-                strnkcpy(chroot_dir, optarg, sizeof chroot_dir);
+                copy_cmdarg(chroot_dir, optarg, sizeof chroot_dir, "chroot");
                 break;
             case 's':
-                strnkcpy(state_dir, optarg, sizeof state_dir);
+                copy_cmdarg(state_dir, optarg, sizeof state_dir, "state-dir");
                 break;
             case 'S':
                 seccomp_enforce = true;
@@ -588,8 +601,8 @@ int main(int argc, char **argv)
                 exit(EXIT_SUCCESS);
                 break;
             case 'V':
-                strnkcpy(client_config.vendor, optarg,
+                copy_cmdarg(client_config.vendor, optarg,
-                         sizeof client_config.vendor);
+                            sizeof client_config.vendor, "vendorid");
                 break;
             case 't': {
                 char *p;
@@ -609,7 +622,8 @@ int main(int argc, char **argv)
                 break;
             }
             case 'R':
-                strnkcpy(resolv_conf_d, optarg, sizeof resolv_conf_d);
+                copy_cmdarg(resolv_conf_d, optarg, sizeof resolv_conf_d,
+                            "resolv-conf");
                 break;
             case 'H':
                 allow_hostname = 1;