diff --git a/include/sudo_iolog.h b/include/sudo_iolog.h index 6ca120cc5..813042569 100644 --- a/include/sudo_iolog.h +++ b/include/sudo_iolog.h @@ -98,12 +98,11 @@ struct iolog_file { struct iolog_path_escape { const char *name; - size_t (*copy_fn)(char *, size_t, char *, void *); + size_t (*copy_fn)(char *, size_t, void *); }; /* iolog_path.c */ -/* XXX - bad API */ -char *expand_iolog_path(const char *prefix, const char *dir, const char *file, char **slashp, const struct iolog_path_escape *escapes, void *closure); +bool expand_iolog_path(const char *inpath, char *path, size_t pathlen, const struct iolog_path_escape *escapes, void *closure); /* iolog_util.c */ bool iolog_parse_timing(const char *line, struct timing_closure *timing); @@ -119,6 +118,7 @@ struct group; bool iolog_close(struct iolog_file *iol, const char **errstr); bool iolog_eof(struct iolog_file *iol); bool iolog_mkdtemp(char *path); +bool iolog_mkpath(char *path); bool iolog_nextid(char *iolog_dir, char sessid[7]); bool iolog_open(struct iolog_file *iol, int dfd, int iofd, const char *mode); bool iolog_rename(const char *from, const char *to); @@ -127,7 +127,6 @@ char *iolog_gets(struct iolog_file *iol, char *buf, size_t nbytes, const char ** const char *iolog_fd_to_name(int iofd); int iolog_openat(int fdf, const char *path, int flags); off_t iolog_seek(struct iolog_file *iol, off_t offset, int whence); -size_t mkdir_iopath(const char *iolog_path, char *pathbuf, size_t pathsize); ssize_t iolog_read(struct iolog_file *iol, void *buf, size_t nbytes, const char **errstr); ssize_t iolog_write(struct iolog_file *iol, const void *buf, size_t len, const char **errstr); void iolog_rewind(struct iolog_file *iol); diff --git a/lib/iolog/iolog_fileio.c b/lib/iolog/iolog_fileio.c index 781d59179..2d1dfdb3c 100644 --- a/lib/iolog/iolog_fileio.c +++ b/lib/iolog/iolog_fileio.c @@ -517,34 +517,28 @@ done: } /* - * Copy iolog_path to pathbuf and create the directory and any intermediate - * directories. If iolog_path ends in 'XXXXXX', use mkdtemp(). - * Returns SIZE_MAX on error. + * Create path and any intermediate directories. + * If path ends in 'XXXXXX', use mkdtemp(). */ -size_t -mkdir_iopath(const char *iolog_path, char *pathbuf, size_t pathsize) +bool +iolog_mkpath(char *path) { size_t len; - bool ok; - debug_decl(mkdir_iopath, SUDO_DEBUG_UTIL) - - len = strlcpy(pathbuf, iolog_path, pathsize); - if (len >= pathsize) { - errno = ENAMETOOLONG; - debug_return_size_t((size_t)-1); - } + bool ret; + debug_decl(iolog_mkpath, SUDO_DEBUG_UTIL) /* * Create path and intermediate subdirs as needed. * If path ends in at least 6 Xs (ala POSIX mktemp), use mkdtemp(). * Sets iolog_gid (if it is not already set) as a side effect. */ - if (len >= 6 && strcmp(&pathbuf[len - 6], "XXXXXX") == 0) - ok = iolog_mkdtemp(pathbuf); + len = strlen(path); + if (len >= 6 && strcmp(&path[len - 6], "XXXXXX") == 0) + ret = iolog_mkdtemp(path); else - ok = iolog_mkdirs(pathbuf); + ret = iolog_mkdirs(path); - debug_return_size_t(ok ? len : (size_t)-1); + debug_return_bool(ret); } /* diff --git a/lib/iolog/iolog_path.c b/lib/iolog/iolog_path.c index 02410fb15..5373e6197 100644 --- a/lib/iolog/iolog_path.c +++ b/lib/iolog/iolog_path.c @@ -52,135 +52,88 @@ #include "sudo_iolog.h" /* - * Concatenate dir + file, expanding any escape sequences. - * Returns the concatenated path and sets slashp point to - * the path separator between the expanded dir and file. - * XXX - simplify by only expanding one thing and removing prefix + * Expand any escape sequences in inpath, returning the expanded path. */ -char * -expand_iolog_path(const char *prefix, const char *dir, const char *file, - char **slashp, const struct iolog_path_escape *escapes, void *closure) +bool +expand_iolog_path(const char *inpath, char *path, size_t pathlen, + const struct iolog_path_escape *escapes, void *closure) { - size_t len, prelen = 0; - char *dst, *dst0, *path, *pathend, tmpbuf[PATH_MAX]; - char *slash = NULL; - const char *endbrace, *src = dir; - int pass; - bool strfit; + char *dst, *pathend, tmpbuf[PATH_MAX]; + const char *endbrace, *src; + bool strfit = false; + size_t len; debug_decl(expand_iolog_path, SUDO_DEBUG_UTIL) - /* Expanded path must be <= PATH_MAX */ - if (prefix != NULL) - prelen = strlen(prefix); - path = malloc(prelen + PATH_MAX); - if (path == NULL) { - sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); - goto bad; - } - *path = '\0'; - pathend = path + prelen + PATH_MAX; - dst = path; + /* Collapse multiple leading slashes. */ + while (inpath[0] == '/' && inpath[1] == '/') + inpath++; - /* Copy prefix, if present. */ - if (prefix != NULL) { - memcpy(path, prefix, prelen); - dst += prelen; - *dst = '\0'; - } - - /* Trim leading slashes from file component. */ - while (*file == '/') - file++; - - for (pass = 0; pass < 3; pass++) { - strfit = false; - switch (pass) { - case 0: - src = dir; - escapes++; /* skip "%{seq}" */ - break; - case 1: - /* Trim trailing slashes from dir component. */ - while (dst > path + prelen + 1 && dst[-1] == '/') - dst--; - /* The NUL will be replaced with a '/' at the end. */ - if (dst + 1 >= pathend) - goto bad; - slash = dst++; - continue; - case 2: - src = file; - escapes--; /* restore "%{seq}" */ - break; - } - dst0 = dst; - for (; *src != '\0'; src++) { - if (src[0] == '%') { - if (src[1] == '{') { - endbrace = strchr(src + 2, '}'); - if (endbrace != NULL) { - const struct iolog_path_escape *esc; - len = (size_t)(endbrace - src - 2); - for (esc = escapes; esc->name != NULL; esc++) { - if (strncmp(src + 2, esc->name, len) == 0 && - esc->name[len] == '\0') - break; - } - if (esc->name != NULL) { - len = esc->copy_fn(dst, (size_t)(pathend - dst), - path + prelen, closure); - if (len >= (size_t)(pathend - dst)) - goto bad; - dst += len; - src = endbrace; - continue; - } + pathend = path + pathlen; + for (src = inpath, dst = path; *src != '\0'; src++) { + if (src[0] == '%') { + if (src[1] == '{') { + endbrace = strchr(src + 2, '}'); + if (endbrace != NULL) { + const struct iolog_path_escape *esc; + len = (size_t)(endbrace - src - 2); + for (esc = escapes; esc->name != NULL; esc++) { + if (strncmp(src + 2, esc->name, len) == 0 && + esc->name[len] == '\0') + break; + } + if (esc->name != NULL) { + len = esc->copy_fn(dst, (size_t)(pathend - dst), + closure); + if (len >= (size_t)(pathend - dst)) + goto bad; + dst += len; + src = endbrace; + continue; } - } else if (src[1] == '%') { - /* Collapse %% -> % */ - src++; - } else { - /* May need strftime() */ - strfit = true; } + } else if (src[1] == '%') { + /* Collapse %% -> % */ + src++; + } else { + /* May need strftime() */ + strfit = true; } - /* Need at least 2 chars, including the NUL terminator. */ - if (dst + 1 >= pathend) - goto bad; - *dst++ = *src; - } - *dst = '\0'; - - /* Expand strftime escapes as needed. */ - if (strfit) { - time_t now; - struct tm *timeptr; - - time(&now); - if ((timeptr = localtime(&now)) == NULL) - goto bad; - - /* We only call strftime() on the current part of the buffer. */ - tmpbuf[sizeof(tmpbuf) - 1] = '\0'; - len = strftime(tmpbuf, sizeof(tmpbuf), dst0, timeptr); - - if (len == 0 || tmpbuf[sizeof(tmpbuf) - 1] != '\0') - goto bad; /* strftime() failed, buf too small? */ - - if (len >= (size_t)(pathend - dst0)) - goto bad; /* expanded buffer too big to fit. */ - memcpy(dst0, tmpbuf, len); - dst = dst0 + len; - *dst = '\0'; } + /* Need at least 2 chars, including the NUL terminator. */ + if (dst + 1 >= pathend) + goto bad; + *dst++ = *src; } - if (slash != NULL) - *slash = '/'; - if (slashp != NULL) - *slashp = slash; - debug_return_str(path); + /* Trim trailing slashes and NUL terminate. */ + while (dst > path && dst[-1] == '/') + dst--; + *dst = '\0'; + + /* Expand strftime escapes as needed. */ + if (strfit) { + time_t now; + struct tm *timeptr; + + time(&now); + if ((timeptr = localtime(&now)) == NULL) + goto bad; + + /* We only call strftime() on the current part of the buffer. */ + tmpbuf[sizeof(tmpbuf) - 1] = '\0'; + len = strftime(tmpbuf, sizeof(tmpbuf), path, timeptr); + + if (len == 0 || tmpbuf[sizeof(tmpbuf) - 1] != '\0') + goto bad; /* strftime() failed, buf too small? */ + + if (len >= (size_t)(pathend - path)) + goto bad; /* expanded buffer too big to fit. */ + memcpy(path, tmpbuf, len); + dst = path + len; + *dst = '\0'; + } + + debug_return_bool(true); bad: - free(path); - debug_return_str(NULL); + debug_return_bool(false); } diff --git a/lib/iolog/regress/iolog_path/check_iolog_path.c b/lib/iolog/regress/iolog_path/check_iolog_path.c index 9035feeed..11f77d444 100644 --- a/lib/iolog/regress/iolog_path/check_iolog_path.c +++ b/lib/iolog/regress/iolog_path/check_iolog_path.c @@ -27,6 +27,7 @@ #ifdef HAVE_STRINGS_H # include #endif /* HAVE_STRINGS_H */ +#include #include #include @@ -69,7 +70,7 @@ reset_escape_data(struct iolog_escape_data *data) } static size_t -fill_seq(char *str, size_t strsize, char *logdir, void *closure) +fill_seq(char *str, size_t strsize, void *unused) { int len; @@ -83,37 +84,37 @@ fill_seq(char *str, size_t strsize, char *logdir, void *closure) } static size_t -fill_user(char *str, size_t strsize, char *unused, void *closure) +fill_user(char *str, size_t strsize, void *unused) { return strlcpy(str, escape_data.user, strsize); } static size_t -fill_group(char *str, size_t strsize, char *unused, void *closure) +fill_group(char *str, size_t strsize, void *unused) { return strlcpy(str, escape_data.group, strsize); } static size_t -fill_runas_user(char *str, size_t strsize, char *unused, void *closure) +fill_runas_user(char *str, size_t strsize, void *unused) { return strlcpy(str, escape_data.runas_user, strsize); } static size_t -fill_runas_group(char *str, size_t strsize, char *unused, void *closure) +fill_runas_group(char *str, size_t strsize, void *unused) { return strlcpy(str, escape_data.runas_group, strsize); } static size_t -fill_hostname(char *str, size_t strsize, char *unused, void *closure) +fill_hostname(char *str, size_t strsize, void *unused) { return strlcpy(str, escape_data.host, strsize); } static size_t -fill_command(char *str, size_t strsize, char *unused, void *closure) +fill_command(char *str, size_t strsize, void *unused) { return strlcpy(str, escape_data.command, strsize); } @@ -133,8 +134,8 @@ static struct iolog_path_escape path_escapes[] = { static int do_check(char *dir_in, char *file_in, char *tdir_out, char *tfile_out) { - char *path, *slash; - char dir_out[4096], file_out[4096]; + char dir[PATH_MAX], dir_out[PATH_MAX]; + char file[PATH_MAX], file_out[PATH_MAX]; struct tm *timeptr; time_t now; int error = 0; @@ -150,19 +151,19 @@ do_check(char *dir_in, char *file_in, char *tdir_out, char *tfile_out) strftime(dir_out, sizeof(dir_out), tdir_out, timeptr); strftime(file_out, sizeof(file_out), tfile_out, timeptr); - path = expand_iolog_path(NULL, dir_in, file_in, &slash, path_escapes, NULL); - if (path == NULL) - sudo_fatalx("unable to expand I/O log path"); - *slash = '\0'; - if (strcmp(path, dir_out) != 0) { - sudo_warnx("%s: expected %s, got %s", dir_in, dir_out, path); + if (!expand_iolog_path(dir_in, dir, sizeof(dir), &path_escapes[1], NULL)) + sudo_fatalx("unable to expand I/O log dir"); + if (!expand_iolog_path(file_in, file, sizeof(file), &path_escapes[0], dir)) + sudo_fatalx("unable to expand I/O log file"); + + if (strcmp(dir, dir_out) != 0) { + sudo_warnx("%s: expected %s, got %s", dir_in, dir_out, dir); error = 1; } - if (strcmp(slash + 1, file_out) != 0) { - sudo_warnx("%s: expected %s, got %s", file_in, file_out, slash + 1); + if (strcmp(file, file_out) != 0) { + sudo_warnx("%s: expected %s, got %s", file_in, file_out, file); error = 1; } - free(path); return error; } diff --git a/lib/iolog/regress/iolog_path/data b/lib/iolog/regress/iolog_path/data index fa4c5b502..48dc87e9d 100644 --- a/lib/iolog/regress/iolog_path/data +++ b/lib/iolog/regress/iolog_path/data @@ -66,9 +66,9 @@ wheel somehost su /var/log/sudo-io/ -/%{user}/%{runas_user}/%{command}_%Y%m%s_%H%M +//%{user}/%{runas_user}/%{command}_%Y%m%s_%H%M /var/log/sudo-io -nobody/root/su_%Y%m%s_%H%M +/nobody/root/su_%Y%m%s_%H%M 000001 nobody @@ -91,6 +91,6 @@ somehost su //////// %{user}/%{runas_user}/%{command} -/ + nobody/root/su diff --git a/logsrvd/iolog_writer.c b/logsrvd/iolog_writer.c index 296f37261..398320ade 100644 --- a/logsrvd/iolog_writer.c +++ b/logsrvd/iolog_writer.c @@ -355,16 +355,21 @@ done: debug_return_bool(ret); } +struct iolog_path_closure { + char *iolog_dir; + struct iolog_details *details; +}; + static size_t -fill_seq(char *str, size_t strsize, char *logdir, void *closure) +fill_seq(char *str, size_t strsize, void *v) { - struct iolog_details *details = closure; - char *sessid = details->sessid; + struct iolog_path_closure *closure = v; + char *sessid = closure->details->sessid; int len; debug_decl(fill_seq, SUDO_DEBUG_UTIL) if (sessid[0] == '\0') { - if (!iolog_nextid(logdir, sessid)) + if (!iolog_nextid(closure->iolog_dir, sessid)) debug_return_size_t((size_t)-1); } @@ -380,9 +385,10 @@ fill_seq(char *str, size_t strsize, char *logdir, void *closure) } static size_t -fill_user(char *str, size_t strsize, char *unused, void *closure) +fill_user(char *str, size_t strsize, void *v) { - const struct iolog_details *details = closure; + struct iolog_path_closure *closure = v; + const struct iolog_details *details = closure->details; debug_decl(fill_user, SUDO_DEBUG_UTIL) if (details->submituser == NULL) { @@ -394,9 +400,10 @@ fill_user(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_group(char *str, size_t strsize, char *unused, void *closure) +fill_group(char *str, size_t strsize, void *v) { - const struct iolog_details *details = closure; + struct iolog_path_closure *closure = v; + const struct iolog_details *details = closure->details; debug_decl(fill_group, SUDO_DEBUG_UTIL) if (details->submitgroup == NULL) { @@ -408,9 +415,10 @@ fill_group(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_runas_user(char *str, size_t strsize, char *unused, void *closure) +fill_runas_user(char *str, size_t strsize, void *v) { - const struct iolog_details *details = closure; + struct iolog_path_closure *closure = v; + const struct iolog_details *details = closure->details; debug_decl(fill_runas_user, SUDO_DEBUG_UTIL) if (details->runuser == NULL) { @@ -422,9 +430,10 @@ fill_runas_user(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_runas_group(char *str, size_t strsize, char *unused, void *closure) +fill_runas_group(char *str, size_t strsize, void *v) { - const struct iolog_details *details = closure; + struct iolog_path_closure *closure = v; + const struct iolog_details *details = closure->details; debug_decl(fill_runas_group, SUDO_DEBUG_UTIL) /* FIXME: rungroup not guaranteed to be set */ @@ -437,9 +446,10 @@ fill_runas_group(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_hostname(char *str, size_t strsize, char *unused, void *closure) +fill_hostname(char *str, size_t strsize, void *v) { - const struct iolog_details *details = closure; + struct iolog_path_closure *closure = v; + const struct iolog_details *details = closure->details; debug_decl(fill_hostname, SUDO_DEBUG_UTIL) if (details->submithost == NULL) { @@ -451,9 +461,10 @@ fill_hostname(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_command(char *str, size_t strsize, char *unused, void *closure) +fill_command(char *str, size_t strsize, void *v) { - const struct iolog_details *details = closure; + struct iolog_path_closure *closure = v; + const struct iolog_details *details = closure->details; debug_decl(fill_command, SUDO_DEBUG_UTIL) if (details->command == NULL) { @@ -476,7 +487,6 @@ static const struct iolog_path_escape path_escapes[] = { { NULL, NULL } }; - /* * Create I/O log path * Sets iolog_path, iolog_file and iolog_dir_fd in the closure @@ -485,38 +495,52 @@ static bool create_iolog_path(struct connection_closure *closure) { struct iolog_details *details = &closure->details; - char pathbuf[PATH_MAX]; - size_t len, pathlen; + struct iolog_path_closure path_closure; + char expanded_dir[PATH_MAX], expanded_file[PATH_MAX], pathbuf[PATH_MAX]; + size_t len; debug_decl(create_iolog_path, SUDO_DEBUG_UTIL) - details->iolog_path = expand_iolog_path(NULL, logsrvd_conf_iolog_dir(), - logsrvd_conf_iolog_file(), &details->iolog_file, &path_escapes[0], - details); - if (details->iolog_path == NULL) { + path_closure.details = details; + path_closure.iolog_dir = expanded_dir; + + if (!expand_iolog_path(logsrvd_conf_iolog_dir(), expanded_dir, + sizeof(expanded_dir), &path_escapes[1], &path_closure)) { sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, - "unable to expand iolog path %s/%s", - logsrvd_conf_iolog_dir(), logsrvd_conf_iolog_file()); + "unable to expand iolog dir %s", logsrvd_conf_iolog_dir()); + goto bad; + } + + if (!expand_iolog_path(logsrvd_conf_iolog_file(), expanded_file, + sizeof(expanded_file), &path_escapes[0], &path_closure)) { + sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, + "unable to expand iolog dir %s", logsrvd_conf_iolog_file()); + goto bad; + } + + len = snprintf(pathbuf, sizeof(pathbuf), "%s/%s", expanded_dir, + expanded_file); + if (len >= sizeof(pathbuf)) { + errno = ENAMETOOLONG; + sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, + "%s/%s", expanded_dir, expanded_file); goto bad; } - pathlen = details->iolog_file - details->iolog_path; /* - * Make local copy of I/O log path and create it, along with any - * intermediate subdirs. Calls mkdtemp() if iolog_path ends in XXXXXX. + * Create log path, along with any intermediate subdirs. + * Calls mkdtemp() if pathbuf ends in XXXXXX. */ - len = mkdir_iopath(details->iolog_path, pathbuf, sizeof(pathbuf)); - if (len >= sizeof(pathbuf)) { + if (!iolog_mkpath(pathbuf)) { sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, - "unable to mkdir iolog path %s", details->iolog_path); + "unable to mkdir iolog path %s", pathbuf); goto bad; } - free(details->iolog_path); if ((details->iolog_path = strdup(pathbuf)) == NULL) { sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, "strdup"); goto bad; } - details->iolog_file = details->iolog_path + pathlen + 1; + details->iolog_file = details->iolog_path + strlen(expanded_dir) + 1; /* We use iolog_dir_fd in calls to openat(2) */ closure->iolog_dir_fd = diff --git a/plugins/sudoers/iolog.c b/plugins/sudoers/iolog.c index 2586d71c7..00bef5f74 100644 --- a/plugins/sudoers/iolog.c +++ b/plugins/sudoers/iolog.c @@ -431,7 +431,6 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation, { struct sudo_conf_debug_file_list debug_files = TAILQ_HEAD_INITIALIZER(debug_files); char iolog_path[PATH_MAX], sessid[7]; - char *tofree = NULL; char * const *cur; const char *cp, *plugin_path = NULL; size_t len; @@ -478,30 +477,36 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation, /* If no I/O log path defined we need to figure it out ourselves. */ if (iolog_details.iolog_path == NULL) { /* Get next session ID and convert it into a path. */ - tofree = malloc(sizeof(_PATH_SUDO_IO_LOGDIR) + sizeof(sessid) + 2); - if (tofree == NULL) { - sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); + len = strlcpy(iolog_path, _PATH_SUDO_IO_LOGDIR, sizeof(iolog_path)); + if (len + strlen("/00/00/00") >= sizeof(iolog_path)) { + sudo_warnx(U_("internal error, %s overflow"), __func__); + ret = false; goto done; } - memcpy(tofree, _PATH_SUDO_IO_LOGDIR, sizeof(_PATH_SUDO_IO_LOGDIR)); - if (!iolog_nextid(tofree, sessid)) { + if (!iolog_nextid(iolog_path, sessid)) { log_warning(SLOG_SEND_MAIL, N_("unable to update sequence file")); ret = false; goto done; } - (void)snprintf(tofree + sizeof(_PATH_SUDO_IO_LOGDIR), - sizeof(sessid) + 2, "%c%c/%c%c/%c%c", sessid[0], sessid[1], - sessid[2], sessid[3], sessid[4], sessid[5]); - iolog_details.iolog_path = tofree; + (void)snprintf(iolog_path + sizeof(_PATH_SUDO_IO_LOGDIR), + sizeof(iolog_path) - sizeof(_PATH_SUDO_IO_LOGDIR), + "/%c%c/%c%c/%c%c", sessid[0], sessid[1], sessid[2], + sessid[3], sessid[4], sessid[5]); + } else { + len = strlcpy(iolog_path, iolog_details.iolog_path, sizeof(iolog_path)); + if (len >= sizeof(iolog_path)) { + sudo_warnx(U_("internal error, %s overflow"), __func__); + ret = false; + goto done; + } } /* - * Make local copy of I/O log path and create it, along with any - * intermediate subdirs. Calls mkdtemp() if iolog_path ends in XXXXXX. + * Create I/O log path along with any * intermediate subdirs. + * Calls mkdtemp() if iolog_path ends in XXXXXX. */ - len = mkdir_iopath(iolog_details.iolog_path, iolog_path, sizeof(iolog_path)); - if (len >= sizeof(iolog_path)) { - log_warning(SLOG_SEND_MAIL, "%s", iolog_details.iolog_path); + if (!iolog_mkpath(iolog_path)) { + log_warning(SLOG_SEND_MAIL, "%s", iolog_path); goto done; } @@ -549,7 +554,6 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation, done: if (iolog_dir_fd != -1) close(iolog_dir_fd); - free(tofree); if (iolog_details.runas_pw) sudo_pw_delref(iolog_details.runas_pw); if (iolog_details.runas_gr) diff --git a/plugins/sudoers/iolog_path_escapes.c b/plugins/sudoers/iolog_path_escapes.c index e2b9d2f99..0f816a7f4 100644 --- a/plugins/sudoers/iolog_path_escapes.c +++ b/plugins/sudoers/iolog_path_escapes.c @@ -42,12 +42,13 @@ #include "sudo_iolog.h" static size_t -fill_seq(char *str, size_t strsize, char *logdir, void *closure) +fill_seq(char *str, size_t strsize, void *v) { #ifdef SUDOERS_NO_SEQ debug_decl(fill_seq, SUDO_DEBUG_UTIL) debug_return_size_t(strlcpy(str, "%{seq}", strsize)); #else + char *logdir = v; static char sessid[7]; int len; debug_decl(fill_seq, SUDO_DEBUG_UTIL) @@ -67,14 +68,14 @@ fill_seq(char *str, size_t strsize, char *logdir, void *closure) } static size_t -fill_user(char *str, size_t strsize, char *unused, void *closure) +fill_user(char *str, size_t strsize, void *unused) { debug_decl(fill_user, SUDO_DEBUG_UTIL) debug_return_size_t(strlcpy(str, user_name, strsize)); } static size_t -fill_group(char *str, size_t strsize, char *unused, void *closure) +fill_group(char *str, size_t strsize, void *unused) { struct group *grp; size_t len; @@ -92,14 +93,14 @@ fill_group(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_runas_user(char *str, size_t strsize, char *unused, void *closure) +fill_runas_user(char *str, size_t strsize, void *unused) { debug_decl(fill_runas_user, SUDO_DEBUG_UTIL) debug_return_size_t(strlcpy(str, runas_pw->pw_name, strsize)); } static size_t -fill_runas_group(char *str, size_t strsize, char *unused, void *closure) +fill_runas_group(char *str, size_t strsize, void *unused) { struct group *grp; size_t len; @@ -121,14 +122,14 @@ fill_runas_group(char *str, size_t strsize, char *unused, void *closure) } static size_t -fill_hostname(char *str, size_t strsize, char *unused, void *closure) +fill_hostname(char *str, size_t strsize, void *unused) { debug_decl(fill_hostname, SUDO_DEBUG_UTIL) debug_return_size_t(strlcpy(str, user_shost, strsize)); } static size_t -fill_command(char *str, size_t strsize, char *unused, void *closure) +fill_command(char *str, size_t strsize, void *unused) { debug_decl(fill_command, SUDO_DEBUG_UTIL) debug_return_size_t(strlcpy(str, user_base, strsize)); diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 2444cc1cf..1e8abd82b 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -233,6 +233,45 @@ cleanup: debug_return_int(ret); } +/* + * Expand I/O log dir and file into a full path. + * Returns the full I/O log path prefixed with "iolog_path=". + * Sets sudo_user.iolog_file as a side effect. + */ +static char * +format_iolog_path(void) +{ + char dir[PATH_MAX], file[PATH_MAX]; + char *iolog_path = NULL; + int oldlocale; + bool ok; + debug_decl(format_iolog_path, SUDOERS_DEBUG_PLUGIN) + + /* Use sudoers locale for strftime() */ + sudoers_setlocale(SUDOERS_LOCALE_SUDOERS, &oldlocale); + ok = expand_iolog_path(def_iolog_dir, dir, sizeof(dir), + &sudoers_iolog_path_escapes[1], NULL); + if (ok) { + ok = expand_iolog_path(def_iolog_file, file, sizeof(file), + &sudoers_iolog_path_escapes[0], dir); + } + sudoers_setlocale(oldlocale, NULL); + if (!ok) + goto done; + + if (asprintf(&iolog_path, "iolog_path=%s/%s", dir, file) == -1) { + iolog_path = NULL; + goto done; + } + + /* Stash pointer to the I/O log file for use when logging. */ + sudo_user.iolog_file = + iolog_path + sizeof("iolog_path=") - 1 + strlen(dir) + 1; + +done: + debug_return_str(iolog_path); +} + int sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], bool verbose, void *closure) @@ -472,21 +511,12 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], if (ISSET(sudo_mode, (MODE_RUN | MODE_EDIT))) { if ((def_log_input || def_log_output) && def_iolog_file && def_iolog_dir) { - const char prefix[] = "iolog_path="; - /* Use sudoers locale for strftime() */ - sudoers_setlocale(SUDOERS_LOCALE_SUDOERS, &oldlocale); - iolog_path = expand_iolog_path(prefix, def_iolog_dir, - def_iolog_file, &sudo_user.iolog_file, - sudoers_iolog_path_escapes, NULL); - sudoers_setlocale(oldlocale, NULL); - if (iolog_path == NULL) { + if ((iolog_path = format_iolog_path()) == NULL) { if (!def_ignore_iolog_errors) goto done; /* Unable to expand I/O log path, disable I/O logging. */ def_log_input = false; def_log_output = false; - } else { - sudo_user.iolog_file++; } } }