From e5652fc65a54ab2d3f161c264254579f00699b00 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Thu, 14 Jul 2022 09:29:40 -0600 Subject: [PATCH] Linux execve(2) allows argv or envp to be NULL. Add checks to make sure we don't deference a NULL pointer. --- lib/util/sudo_debug.c | 10 +++++----- src/exec_intercept.c | 2 +- src/exec_preload.c | 5 +++++ src/exec_ptrace.c | 36 +++++++++++++++++++++++++++++------- src/sudo_intercept.c | 10 ++++++++++ src/sudo_intercept_common.c | 25 ++++++++++++++----------- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/lib/util/sudo_debug.c b/lib/util/sudo_debug.c index d78536a0c..a2ead5228 100644 --- a/lib/util/sudo_debug.c +++ b/lib/util/sudo_debug.c @@ -831,7 +831,7 @@ sudo_debug_execve2_v1(int level, const char *path, char *const argv[], char *con size_t plen; debug_decl_func(sudo_debug_execve2); - if (sudo_debug_active_instance == -1) + if (sudo_debug_active_instance == -1 || path == NULL) goto out; /* Extract priority and subsystem from level. */ @@ -867,13 +867,13 @@ sudo_debug_execve2_v1(int level, const char *path, char *const argv[], char *con /* Alloc and build up buffer. */ plen = strlen(path); buflen = sizeof(EXEC_PREFIX) -1 + plen; - if (argv[0] != NULL) { + if (argv != NULL && argv[0] != NULL) { buflen += sizeof(" []") - 1; for (av = argv; *av; av++) buflen += strlen(*av) + 1; buflen--; } - if (log_envp) { + if (envp != NULL && log_envp) { buflen += sizeof(" []") - 1; for (av = envp; *av; av++) buflen += strlen(*av) + 1; @@ -892,7 +892,7 @@ sudo_debug_execve2_v1(int level, const char *path, char *const argv[], char *con cp += plen; /* Copy argv. */ - if (argv[0] != NULL) { + if (argv != NULL && argv[0] != NULL) { *cp++ = ' '; *cp++ = '['; for (av = argv; *av; av++) { @@ -904,7 +904,7 @@ sudo_debug_execve2_v1(int level, const char *path, char *const argv[], char *con cp[-1] = ']'; } - if (log_envp) { + if (envp != NULL && log_envp) { *cp++ = ' '; *cp++ = '['; for (av = envp; *av; av++) { diff --git a/src/exec_intercept.c b/src/exec_intercept.c index ea429e339..6017d6481 100644 --- a/src/exec_intercept.c +++ b/src/exec_intercept.c @@ -491,7 +491,7 @@ intercept_check_policy_req(PolicyCheckRequest *req, size_t n; debug_decl(intercept_check_policy_req, SUDO_DEBUG_EXEC); - if (req->command == NULL || req->n_argv == 0 || req->n_envp == 0) { + if (req->command == NULL || req->n_argv == 0) { closure->errstr = N_("invalid PolicyCheckRequest"); goto done; } diff --git a/src/exec_preload.c b/src/exec_preload.c index 4f04a0113..81e865333 100644 --- a/src/exec_preload.c +++ b/src/exec_preload.c @@ -44,6 +44,7 @@ sudo_preload_dso(char *const envp[], const char *dso_file, int intercept_fd) char *const *ep; char **preload_ptr = NULL; char **intercept_ptr = NULL; + char *const empty[1] = { NULL }; bool fd_present = false; bool dso_present = false; # ifdef RTLD_PRELOAD_ENABLE_VAR @@ -75,6 +76,10 @@ sudo_preload_dso(char *const envp[], const char *dso_file, int intercept_fd) * XXX - need to support 32-bit and 64-bit variants */ + /* Treat a NULL envp as empty, thanks Linux. */ + if (envp == NULL) + envp = empty; + /* Determine max size for new envp. */ for (env_size = 0; envp[env_size] != NULL; env_size++) continue; diff --git a/src/exec_ptrace.c b/src/exec_ptrace.c index c6f948501..4948679e8 100644 --- a/src/exec_ptrace.c +++ b/src/exec_ptrace.c @@ -386,6 +386,12 @@ ptrace_read_vec(pid_t pid, struct sudo_ptrace_regs *regs, unsigned long addr, size_t slen; debug_decl(ptrace_read_vec, SUDO_DEBUG_EXEC); + /* Treat a NULL vector as empty, thanks Linux. */ + if (addr == 0) { + vec[0] = NULL; + debug_return_size_t(0); + } + /* Fill in vector. */ for (;;) { # ifdef SECCOMP_AUDIT_ARCH_COMPAT @@ -443,6 +449,10 @@ ptrace_get_vec_len(pid_t pid, struct sudo_ptrace_regs *regs, unsigned long addr) int len = 0; debug_decl(ptrace_get_vec_len, SUDO_DEBUG_EXEC); + /* Treat a NULL vector as empty, thanks Linux. */ + if (addr == 0) + debug_return_int(0); + for (;;) { # ifdef SECCOMP_AUDIT_ARCH_COMPAT if (next_word == (unsigned long)-1) { @@ -634,7 +644,7 @@ get_execve_info(pid_t pid, struct sudo_ptrace_regs *regs, char **pathname_out, argv_addr = get_sc_arg2(regs); envp_addr = get_sc_arg3(regs); - /* Count argv and envp */ + /* Count argv and envp. */ argc = ptrace_get_vec_len(pid, regs, argv_addr); envc = ptrace_get_vec_len(pid, regs, envp_addr); if (argc == -1 || envc == -1) @@ -643,6 +653,7 @@ get_execve_info(pid_t pid, struct sudo_ptrace_regs *regs, char **pathname_out, /* Reserve argv and envp at the start of argbuf so they are aligned. */ if ((argc + 1 + envc + 1) * sizeof(unsigned long) >= bufsize) { sudo_warnx("%s", U_("insufficient space for execve arguments")); + errno = ENOMEM; goto bad; } argv = (char **)argbuf; @@ -671,13 +682,18 @@ get_execve_info(pid_t pid, struct sudo_ptrace_regs *regs, char **pathname_out, bufsize -= len; /* Read the pathname. */ - len = ptrace_read_string(pid, path_addr, strtab, bufsize); - if (len == (size_t)-1) { - sudo_warn(U_("unable to read execve %s for process %d"), - "pathname", (int)pid); - goto bad; + if (path_addr == 0) { + /* execve(2) will fail with EINVAL */ + pathname = NULL; + } else { + len = ptrace_read_string(pid, path_addr, strtab, bufsize); + if (len == (size_t)-1) { + sudo_warn(U_("unable to read execve %s for process %d"), + "pathname", (int)pid); + goto bad; + } + pathname = strtab; } - pathname = strtab; sudo_debug_execve(SUDO_DEBUG_DIAG, pathname, argv, envp); @@ -1071,6 +1087,12 @@ ptrace_intercept_execve(pid_t pid, struct intercept_closure *closure) debug_return_bool(false); } + /* Must have a pathname. */ + if (pathname == NULL) { + ptrace_fail_syscall(pid, ®s, EINVAL); + goto done; + } + /* * Short-circuit the policy check if the command doesn't exist. * Otherwise, both sudo and the shell will report the error. diff --git a/src/sudo_intercept.c b/src/sudo_intercept.c index 97c612892..48ecc9bc9 100644 --- a/src/sudo_intercept.c +++ b/src/sudo_intercept.c @@ -135,6 +135,11 @@ exec_wrapper(const char *cmnd, char * const argv[], char * const envp[], void *fn = NULL; debug_decl(exec_wrapper, SUDO_DEBUG_EXEC); + if (cmnd == NULL) { + errno = EINVAL; + debug_return_int(-1); + } + /* Only check PATH for the command for execlp/execvp/execvpe. */ if (strchr(cmnd, '/') == NULL) { if (!is_execvp) { @@ -201,6 +206,11 @@ execl_wrapper(int type, const char *name, const char *arg, va_list ap) va_list ap2; debug_decl(execl_wrapper, SUDO_DEBUG_EXEC); + if (name == NULL || arg == NULL) { + errno = EINVAL; + debug_return_int(-1); + } + va_copy(ap2, ap); while (va_arg(ap2, char *) != NULL) argc++; diff --git a/src/sudo_intercept_common.c b/src/sudo_intercept_common.c index 102393952..a81a3c880 100644 --- a/src/sudo_intercept_common.c +++ b/src/sudo_intercept_common.c @@ -297,6 +297,7 @@ send_policy_check_req(int sock, const char *cmnd, char * const argv[], InterceptRequest msg = INTERCEPT_REQUEST__INIT; PolicyCheckRequest req = POLICY_CHECK_REQUEST__INIT; char cwdbuf[PATH_MAX]; + char *empty[1] = { NULL }; uint8_t *buf = NULL; bool ret = false; uint32_t msg_len; @@ -313,14 +314,14 @@ send_policy_check_req(int sock, const char *cmnd, char * const argv[], /* Setup policy check request. */ req.intercept_fd = sock; req.command = (char *)cmnd; - req.argv = (char **)argv; - for (len = 0; argv[len] != NULL; len++) - continue; - req.n_argv = len; - req.envp = (char **)envp; - for (len = 0; envp[len] != NULL; len++) - continue; - req.n_envp = len; + req.argv = argv ? (char **)argv : empty; + req.n_argv = 0; + while (req.argv[req.n_argv] != NULL) + req.n_argv++; + req.envp = envp ? (char **)envp : empty; + req.n_envp = 0; + while (req.envp[req.n_envp] != NULL) + req.n_envp++; if (getcwd(cwdbuf, sizeof(cwdbuf)) != NULL) { req.cwd = cwdbuf; } @@ -409,9 +410,11 @@ command_allowed(const char *cmnd, char * const argv[], if (sudo_debug_needed(SUDO_DEBUG_INFO)) { sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, "req_command: %s", cmnd); - for (idx = 0; argv[idx] != NULL; idx++) { - sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, - "req_argv[%zu]: %s", idx, argv[idx]); + if (argv != NULL) { + for (idx = 0; argv[idx] != NULL; idx++) { + sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, + "req_argv[%zu]: %s", idx, argv[idx]); + } } }