From 9e35addc358375c9228fcc2d5df780e9f4fef164 Mon Sep 17 00:00:00 2001 From: Bernd Schubert Date: Tue, 5 Mar 2024 18:09:33 +0100 Subject: [PATCH] fusermount: Fix head-buffer-overflow in extract_x_options Commit 74b1df2e introduced a heap-buffer-overflow, as allocated memory was not initialized and extract_x_options was also not checking for the remaining buffer size. Fix is to initialize the buffer and to also not exceed the buffer size. Actually not exceeding buffer size is rather complex with C and introduced quite some code changes. Also fixed is a memory leak of allocated buffers in the commit mentioned above. --- util/fusermount.c | 113 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 84 insertions(+), 29 deletions(-) diff --git a/util/fusermount.c b/util/fusermount.c index fe75631..06f5f56 100644 --- a/util/fusermount.c +++ b/util/fusermount.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "fuse_mount_compat.h" @@ -100,26 +101,71 @@ static struct mntent *GETMNTENT(FILE *stream) /* * Take a ',' separated option string and extract "x-" options */ -static void extract_x_options(const char *original, char *non_x_opts, char *x_opts) +static int extract_x_options(const char *original, char **non_x_opts, + char **x_opts) { - size_t len; + size_t orig_len; const char *opt, *opt_end; - char *opt_buf; - len = strlen(original); + orig_len = strlen(original) + 1; + + *non_x_opts = calloc(1, orig_len); + *x_opts = calloc(1, orig_len); + + size_t non_x_opts_len = orig_len; + size_t x_opts_len = orig_len; + + if (*non_x_opts == NULL || *x_opts == NULL) { + fprintf(stderr, "%s: Failed to allocate %zuB.\n", + __func__, orig_len); + return -ENOMEM; + } + + for (opt = original; opt < original + orig_len; opt = opt_end + 1) { + char *opt_buf; - for (opt = original; opt < original + len; opt = opt_end + 1) { opt_end = strchr(opt, ','); if (opt_end == NULL) - opt_end = original + len; + opt_end = original + orig_len; - opt_buf = strncmp(opt, "x-", 2) ? non_x_opts : x_opts; + size_t opt_len = opt_end - opt; + size_t opt_len_left = orig_len - (opt - original); + size_t buf_len; + bool is_x_opts; - if (strlen(opt_buf) > 1) + if (strncmp(opt, "x-", MIN(2, opt_len_left)) == 0) { + buf_len = x_opts_len; + is_x_opts = true; + opt_buf = *x_opts; + } else { + buf_len = non_x_opts_len; + is_x_opts = false; + opt_buf = *non_x_opts; + } + + if (buf_len < orig_len) { strncat(opt_buf, ",", 2); + buf_len -= 1; + } + + /* omits ',' */ + if ((ssize_t)(buf_len - opt_len) < 0) { + /* This would be a bug */ + fprintf(stderr, "%s: no buf space left in copy, orig='%s'\n", + __func__, original); + return -EIO; + } strncat(opt_buf, opt, opt_end - opt); + buf_len -= opt_len; + + if (is_x_opts) + x_opts_len = buf_len; + else + non_x_opts_len = buf_len; } + + return 0; } static const char *get_user_name(void) @@ -1159,11 +1205,8 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type) char *mnt_opts = NULL; const char *real_mnt = mnt; int mountpoint_fd = -1; - size_t opts_size; - char *do_mount_opts; - char *x_opts; - size_t add_mount_opts_size, add_mount_opts_len; - char *add_mount_opts; + char *do_mount_opts = NULL; + char *x_opts = NULL; fd = open_fuse_device(&dev); if (fd == -1) @@ -1181,10 +1224,9 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type) } // Extract any options starting with "x-" - opts_size = strlen(opts) + 1; - do_mount_opts = malloc(opts_size); - x_opts = malloc(opts_size); - extract_x_options(opts, do_mount_opts, x_opts); + res= extract_x_options(opts, &do_mount_opts, &x_opts); + if (res) + goto fail_close_fd; res = check_perm(&real_mnt, &stbuf, &mountpoint_fd); restore_privs(); @@ -1205,18 +1247,29 @@ static int mount_fuse(const char *mnt, const char *opts, const char **type) } if (geteuid() == 0) { - // Add back the options starting with "x-" to opts from do_mount - add_mount_opts_size = strlen(mnt_opts) + strlen(x_opts) + 2; - add_mount_opts = malloc(add_mount_opts_size); - - strncpy(add_mount_opts, mnt_opts, add_mount_opts_size - 1); - add_mount_opts_len = strlen(add_mount_opts); - if (add_mount_opts_len > 0) - strncat(add_mount_opts, ",", 2); - strncat(add_mount_opts, x_opts, - add_mount_opts_size - add_mount_opts_len - 2); - - res = add_mount(source, mnt, *type, add_mount_opts); + if (x_opts && strlen(x_opts) > 0) { + /* + * Add back the options starting with "x-" to opts from + * do_mount. +2 for ',' and '\0' + */ + size_t mnt_opts_len = strlen(mnt_opts); + size_t x_mnt_opts_len = mnt_opts_len+ + strlen(x_opts) + 2; + char *x_mnt_opts = malloc(x_mnt_opts_len); + + if (mnt_opts_len) { + strcpy(x_mnt_opts, mnt_opts); + strncat(x_mnt_opts, ",", 2); + } + + strncat(x_mnt_opts, x_opts, + x_mnt_opts_len - mnt_opts_len - 2); + + free(mnt_opts); + mnt_opts = x_mnt_opts; + } + + res = add_mount(source, mnt, *type, mnt_opts); if (res == -1) { /* Can't clean up mount in a non-racy way */ goto fail_close_fd; @@ -1227,6 +1280,8 @@ out_free: free(source); free(mnt_opts); free(dev); + free(x_opts); + free(do_mount_opts); return fd; -- 2.30.2