Fix string truncation warnings related to PATH_MAX

There are a number of places where rrdtool combines multiple PATH_MAX
sized strings into one.

PATH_MAX is a constant that tends to work in practice a lot of the time
but may not reflect the real capabilities of the system in real time.

In place of on-stack buffers of PATH_MAX size allocate memory
dynamically. Initialize the pointers to NULL so they can be all freed
unconditionally on exit.

Fixes: #1223
Signed-off-by: Michal Suchanek <[email protected]>

src/Makefile.am

@@ -71,6 +71,7 @@ noinst_HEADERS = \
 	unused.h \
 	gettext.h \
 	mutex.h \
+	compat-asprintf.h \
 	compat-cloexec.h \
 	rrd_strtod.h \
 	rrd_snprintf.h \

src/compat-asprintf.h

@@ -0,0 +1,35 @@
+#ifndef H_RRDTOOL_SRC_COMPAT_ASPRINTF_H
+#define H_RRDTOOL_SRC_COMPAT_ASPRINTF_H
+
+#include <rrd_config.h>
+#include <stdio.h>
+
+#ifndef HAVE_ASPRINTF
+#include <stdarg.h>
+
+static inline
+int asprintf(char **result, const char *format, ...)
+{
+	va_list ap;
+	va_list aq;
+	int size;
+	char *buffer;
+
+	va_start(ap, format);
+	va_copy(aq, ap);
+	size = vsnprintf(NULL, 0, format, ap);
+	if (size < 0)
+		return size;
+	size++;
+	buffer = malloc(size);
+	if (!buffer)
+		return -ENOMEM;
+	*result = buffer;
+	size = vsnprintf(buffer, size, format, aq);
+	va_end(ap);
+	return size;
+}
+
+#endif	/* HAVE_ASPRINTF */
+
+#endif	/* H_RRDTOOL_SRC_COMPAT_ASPRINTF_H */

src/rrd_daemon.c

@@ -112,6 +112,7 @@
 
 #include "rrd_strtod.h"
 #include "compat-cloexec.h"
+#include "compat-asprintf.h"
 
 #include <glib.h>
 /* }}} */
@@ -128,6 +129,22 @@
 #define RRD_LISTEN_BACKLOG 511
 #endif
 
+static inline ssize_t areadlink(char **result, const char *path) {
+    struct stat st;
+    char * buf;
+    ssize_t size;
+    if (lstat(path, &st) == -1)
+        return -errno;
+    if ((st.st_mode & S_IFMT) != S_IFLNK)
+        return 0;
+    size = st.st_size;
+    buf = calloc(1, size + 1);
+    if (!buf)
+        return -ENOMEM;
+    *result = buf;
+    return readlink(path, buf, size);
+}
+
 /*
  * Types
  */
@@ -2628,12 +2645,14 @@ static int handle_request_list(
     char     *filename = NULL;
     char     *rec = NULL;
     int       recursive = 0;
-    char     *list, *start_ptr, *end_ptr, *ptr;
-    char      fullpath[PATH_MAX], current[PATH_MAX], absolute[PATH_MAX];
-    char      bwc[PATH_MAX], bwd[PATH_MAX];
+    char     *list = NULL;
+    char     *start_ptr, *end_ptr, *ptr;
+    char     *fullpath = NULL;
+    char     *current = NULL;
+    char     *absolute = NULL;
+    char     *bwc = NULL;
+    char     *bwd = NULL;
     char     *base = &config_base_dir[0];
-    struct stat sc, sd;
-    ssize_t   len;
     int       status;
 
     if (config_base_dir == NULL) {
@@ -2665,39 +2684,38 @@ static int handle_request_list(
     }
 
     /* get full pathname */
-    snprintf(fullpath, PATH_MAX, "%s%s%s",
+    status = asprintf(&fullpath, "%s%s%s",
              config_base_dir, (filename[0] == '/') ? "" : "/", filename);
+    if (status < 0)
+        goto out_status_return;
 
     if (!check_file_access(fullpath, sock)) {
-        return send_response(sock, RESP_ERR, "Cannot read: %s\n", fullpath);
+        status = send_response(sock, RESP_ERR, "Cannot read: %s\n", fullpath);
+        goto out_status_return;
     }
 
     /* get real path of config_base_dir in case it's a symlink */
-    if (lstat(config_base_dir, &sd) == -1) {
-        return send_response(sock, RESP_ERR, "stat %s: %s\n",
+    status = areadlink(&bwd, config_base_dir);
+    if (status < 0) {
+        status = send_response(sock, RESP_ERR, "stat %s: %s\n",
                              config_base_dir, rrd_strerror(errno));
+        goto out_status_return;
     }
-
-    if ((sd.st_mode & S_IFMT) == S_IFLNK) {
-        len = readlink(config_base_dir, bwd, sizeof(bwd) - 1);
-        if (len == -1) {
-            return send_response(sock, RESP_ERR, "readlink %s: %s\n",
-                                 config_base_dir, rrd_strerror(errno));
-        }
-        bwd[len] = '\0';
-        base = &bwd[0];
-    }
+    if (status > 0)
+        base = bwd;
 
     list = rrd_list_r(recursive, fullpath);
 
     if (list == NULL) {
         /* Empty directory listing */
         if (errno == 0) {
+            status = 0;
             goto out_send_response;
         }
 
-        return send_response(sock, RESP_ERR,
+        status = send_response(sock, RESP_ERR,
                              "List %s: %s\n", fullpath, rrd_strerror(errno));
+        goto out_status_return;
     }
 
     /* Check list items returned by rrd_list_r;
@@ -2707,6 +2725,15 @@ static int handle_request_list(
     end_ptr = list;
 
     while (*start_ptr != '\0') {
+        char * fn;
+
+        free(absolute);
+        absolute = NULL;
+        free(current);
+        current = NULL;
+        free(bwc);
+        bwc = NULL;
+
         end_ptr = strchr(start_ptr, '\n');
 
         if (end_ptr == NULL) {
@@ -2717,63 +2744,60 @@ static int handle_request_list(
             /* Name too long: skip entry */
             goto loop_next;
         }
-        strncpy(&current[0], start_ptr, (end_ptr - start_ptr));
-        current[end_ptr - start_ptr] = '\0';
+        status = asprintf(&current, "%.*s", (int)(end_ptr - start_ptr), start_ptr);
+        if (status < 0)
+            goto out_status_return;
 
         /* if a single .rrd was asked for, absolute == fullpath  */
         ptr = strstr(fullpath, ".rrd");
 
         if (ptr != NULL && strlen(ptr) == 4) {
-            snprintf(&absolute[0], PATH_MAX, "%s", fullpath);
-
+            fn = fullpath;
         } else {
-            snprintf(&absolute[0], PATH_MAX, "%s/%s", fullpath, current);
+            status = asprintf(&absolute, "%s/%s", fullpath, current);
+            if (status < 0)
+                goto out_status_return;
+            fn = absolute;
         }
 
-        if (!check_file_access(absolute, sock)) {
+        if (!check_file_access(fn, sock)) {
             /* Cannot access: skip entry */
             goto loop_next;
         }
 
         /* Make sure we aren't following a symlink pointing outside of base_dir */
-        if (lstat(absolute, &sc) == -1) {
-            free(list);
-            return send_response(sock, RESP_ERR,
-                                 "stat %s: %s\n", absolute,
+        status = areadlink(&bwc, fn);
+        if (status < 0) {
+            status = send_response(sock, RESP_ERR,
+                                 "stat %s: %s\n", fn,
                                  rrd_strerror(errno));
+            goto out_status_return;
         }
-
-        if ((sc.st_mode & S_IFMT) == S_IFLNK) {
-            len = readlink(absolute, bwc, sizeof(bwc) - 1);
-
-            if (len == -1) {
-                free(list);
-                return send_response(sock, RESP_ERR, "readlink %s: %s\n",
-                                     absolute, rrd_strerror(errno));
-            }
-            bwc[len] = '\0';
-            strncpy(&absolute[0], bwc, PATH_MAX - 1);
-            absolute[PATH_MAX - 1] = '\0';
-        }
+        if (status > 0)
+            fn = bwc;
 
         /* Absolute path MUST be starting with base_dir; if not skip the entry. */
-        if (strlen(absolute) < strlen(base) ||
-            memcmp(absolute, base, strlen(base)) != 0) {
+        if (strlen(fn) < strlen(base) ||
+            memcmp(fn, base, strlen(base)) != 0) {
             goto loop_next;
         }
         add_response_info(sock, "%s\n", current);
 
       loop_next:
         start_ptr = end_ptr + 1;
-
     }
 
-    free(list);
-
   out_send_response:
     send_response(sock, RESP_OK, "RRDs\n");
-
-    return (0);
+    status = 0;
+  out_status_return:
+    free(list);
+    free(fullpath);
+    free(current);
+    free(absolute);
+    free(bwd);
+    free(bwc);
+    return status;
 }                       /* }}} int handle_request_list */
 
 static cache_item_t *buffer_get_cache_item(

src/rrd_list.c

@@ -18,6 +18,7 @@
 
 #include "rrd_tool.h"
 #include "rrd_client.h"
+#include "compat-asprintf.h"
 
 static char *move_past_prefix(const char *prefix, const char *string)
 {
@@ -53,7 +54,7 @@ static char *rrd_list_rec(int recursive, const char *root, const char *dirname)
 	struct dirent *entry;
 	DIR *dir;
 	char *out = NULL, *out_rec, *out_short, *tmp, *ptr;
-	char current[PATH_MAX], fullpath[PATH_MAX];
+	char *current = NULL, *fullpath = NULL;
 
 	dir = opendir(dirname);
 
@@ -63,6 +64,10 @@ static char *rrd_list_rec(int recursive, const char *root, const char *dirname)
 	}
 
 	while ((entry = readdir(dir)) != NULL) {
+		free(current);
+		current = NULL;
+		free(fullpath);
+		fullpath = NULL;
 
 		if ((strcmp(entry->d_name, ".") == 0) ||
 		    (strcmp(entry->d_name, "..") == 0)) {
@@ -73,16 +78,16 @@ static char *rrd_list_rec(int recursive, const char *root, const char *dirname)
 			continue;
 		}
 
-		snprintf(&current[0], PATH_MAX, "%s/%s", dirname, entry->d_name);
+		if (asprintf(&current, "%s/%s", dirname, entry->d_name) < 0)
+			continue;
 
 		/* NOTE: stat(2) follows symlinks and gives info on target. */
 		if (stat(current, &st) != 0) {
 			continue;
 		}
 
 		if (S_ISDIR(st.st_mode) && recursive) {
-			snprintf(&fullpath[0], PATH_MAX, "%s/%s",
-				 dirname, entry->d_name);
+			asprintf(&fullpath, "%s/%s", dirname, entry->d_name);
 			out_rec = rrd_list_rec(recursive, root, fullpath);
 
 			if (out_rec == NULL) {
@@ -113,8 +118,7 @@ static char *rrd_list_rec(int recursive, const char *root, const char *dirname)
 					continue;
 				}
 			}
-			snprintf(&fullpath[0], PATH_MAX, "%s/%s",
-				 dirname, entry->d_name);
+			asprintf(&fullpath,"%s/%s", dirname, entry->d_name);
 			out_short = move_past_prefix(root, fullpath);
 
 			/* don't start output with a '/' */
@@ -133,6 +137,8 @@ static char *rrd_list_rec(int recursive, const char *root, const char *dirname)
 		}
 	}
 	closedir(dir);
+	free(current);
+	free(fullpath);
 
 	errno = 0;
 	return out;