Skip to content

Commit

Permalink
Fix string truncation warnings related to PATH_MAX
Browse files Browse the repository at this point in the history 
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: oetiker#1223
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
  • Loading branch information
@hramrach
hramrach committed Jan 8, 2024
1 parent 6892508 commit 4449577
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 56 deletions.
1 change: 1 addition & 0 deletions CHANGES
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Bugfixes
* acinclude.m4: Include <stdlib.h> when using exit <ryandesign>
* rrdtool-release: Create NUMVERS from VERSION file <c72578>
* Avoids leaking of file descriptors in multi threaded programs by @ensc
* Avoids potential unterminated string because of fixed PATH_MAX buffer

Features
--------
Expand Down
124 changes: 74 additions & 50 deletions src/rrd_daemon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@

#include "rrd_strtod.h"
#include "compat-cloexec.h"
#include "rrd_snprintf.h"

#include <glib.h>
/* }}} */
Expand All @@ -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
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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(
Expand Down
18 changes: 12 additions & 6 deletions src/rrd_list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "rrd_tool.h"
#include "rrd_client.h"
#include "rrd_snprintf.h"

static char *move_past_prefix(const char *prefix, const char *string)
{
Expand Down Expand Up @@ -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);

Expand All @@ -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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 '/' */
Expand All @@ -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;
Expand Down

0 comments on commit 4449577

Please sign in to comment.