Linux execve(2) allows argv or envp to be NULL.

Add checks to make sure we don't deference a NULL pointer.
This commit is contained in:
Todd C. Miller
2022-07-14 09:29:40 -06:00
parent a5ac29219a
commit e5652fc65a
6 changed files with 64 additions and 24 deletions

View File

@@ -831,7 +831,7 @@ sudo_debug_execve2_v1(int level, const char *path, char *const argv[], char *con
size_t plen; size_t plen;
debug_decl_func(sudo_debug_execve2); debug_decl_func(sudo_debug_execve2);
if (sudo_debug_active_instance == -1) if (sudo_debug_active_instance == -1 || path == NULL)
goto out; goto out;
/* Extract priority and subsystem from level. */ /* 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. */ /* Alloc and build up buffer. */
plen = strlen(path); plen = strlen(path);
buflen = sizeof(EXEC_PREFIX) -1 + plen; buflen = sizeof(EXEC_PREFIX) -1 + plen;
if (argv[0] != NULL) { if (argv != NULL && argv[0] != NULL) {
buflen += sizeof(" []") - 1; buflen += sizeof(" []") - 1;
for (av = argv; *av; av++) for (av = argv; *av; av++)
buflen += strlen(*av) + 1; buflen += strlen(*av) + 1;
buflen--; buflen--;
} }
if (log_envp) { if (envp != NULL && log_envp) {
buflen += sizeof(" []") - 1; buflen += sizeof(" []") - 1;
for (av = envp; *av; av++) for (av = envp; *av; av++)
buflen += strlen(*av) + 1; buflen += strlen(*av) + 1;
@@ -892,7 +892,7 @@ sudo_debug_execve2_v1(int level, const char *path, char *const argv[], char *con
cp += plen; cp += plen;
/* Copy argv. */ /* Copy argv. */
if (argv[0] != NULL) { if (argv != NULL && argv[0] != NULL) {
*cp++ = ' '; *cp++ = ' ';
*cp++ = '['; *cp++ = '[';
for (av = argv; *av; av++) { 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] = ']'; cp[-1] = ']';
} }
if (log_envp) { if (envp != NULL && log_envp) {
*cp++ = ' '; *cp++ = ' ';
*cp++ = '['; *cp++ = '[';
for (av = envp; *av; av++) { for (av = envp; *av; av++) {

View File

@@ -491,7 +491,7 @@ intercept_check_policy_req(PolicyCheckRequest *req,
size_t n; size_t n;
debug_decl(intercept_check_policy_req, SUDO_DEBUG_EXEC); 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"); closure->errstr = N_("invalid PolicyCheckRequest");
goto done; goto done;
} }

View File

@@ -44,6 +44,7 @@ sudo_preload_dso(char *const envp[], const char *dso_file, int intercept_fd)
char *const *ep; char *const *ep;
char **preload_ptr = NULL; char **preload_ptr = NULL;
char **intercept_ptr = NULL; char **intercept_ptr = NULL;
char *const empty[1] = { NULL };
bool fd_present = false; bool fd_present = false;
bool dso_present = false; bool dso_present = false;
# ifdef RTLD_PRELOAD_ENABLE_VAR # 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 * 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. */ /* Determine max size for new envp. */
for (env_size = 0; envp[env_size] != NULL; env_size++) for (env_size = 0; envp[env_size] != NULL; env_size++)
continue; continue;

View File

@@ -386,6 +386,12 @@ ptrace_read_vec(pid_t pid, struct sudo_ptrace_regs *regs, unsigned long addr,
size_t slen; size_t slen;
debug_decl(ptrace_read_vec, SUDO_DEBUG_EXEC); 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. */ /* Fill in vector. */
for (;;) { for (;;) {
# ifdef SECCOMP_AUDIT_ARCH_COMPAT # 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; int len = 0;
debug_decl(ptrace_get_vec_len, SUDO_DEBUG_EXEC); 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 (;;) { for (;;) {
# ifdef SECCOMP_AUDIT_ARCH_COMPAT # ifdef SECCOMP_AUDIT_ARCH_COMPAT
if (next_word == (unsigned long)-1) { 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); argv_addr = get_sc_arg2(regs);
envp_addr = get_sc_arg3(regs); envp_addr = get_sc_arg3(regs);
/* Count argv and envp */ /* Count argv and envp. */
argc = ptrace_get_vec_len(pid, regs, argv_addr); argc = ptrace_get_vec_len(pid, regs, argv_addr);
envc = ptrace_get_vec_len(pid, regs, envp_addr); envc = ptrace_get_vec_len(pid, regs, envp_addr);
if (argc == -1 || envc == -1) 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. */ /* Reserve argv and envp at the start of argbuf so they are aligned. */
if ((argc + 1 + envc + 1) * sizeof(unsigned long) >= bufsize) { if ((argc + 1 + envc + 1) * sizeof(unsigned long) >= bufsize) {
sudo_warnx("%s", U_("insufficient space for execve arguments")); sudo_warnx("%s", U_("insufficient space for execve arguments"));
errno = ENOMEM;
goto bad; goto bad;
} }
argv = (char **)argbuf; argv = (char **)argbuf;
@@ -671,13 +682,18 @@ get_execve_info(pid_t pid, struct sudo_ptrace_regs *regs, char **pathname_out,
bufsize -= len; bufsize -= len;
/* Read the pathname. */ /* Read the pathname. */
len = ptrace_read_string(pid, path_addr, strtab, bufsize); if (path_addr == 0) {
if (len == (size_t)-1) { /* execve(2) will fail with EINVAL */
sudo_warn(U_("unable to read execve %s for process %d"), pathname = NULL;
"pathname", (int)pid); } else {
goto bad; 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); 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); debug_return_bool(false);
} }
/* Must have a pathname. */
if (pathname == NULL) {
ptrace_fail_syscall(pid, &regs, EINVAL);
goto done;
}
/* /*
* Short-circuit the policy check if the command doesn't exist. * Short-circuit the policy check if the command doesn't exist.
* Otherwise, both sudo and the shell will report the error. * Otherwise, both sudo and the shell will report the error.

View File

@@ -135,6 +135,11 @@ exec_wrapper(const char *cmnd, char * const argv[], char * const envp[],
void *fn = NULL; void *fn = NULL;
debug_decl(exec_wrapper, SUDO_DEBUG_EXEC); 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. */ /* Only check PATH for the command for execlp/execvp/execvpe. */
if (strchr(cmnd, '/') == NULL) { if (strchr(cmnd, '/') == NULL) {
if (!is_execvp) { if (!is_execvp) {
@@ -201,6 +206,11 @@ execl_wrapper(int type, const char *name, const char *arg, va_list ap)
va_list ap2; va_list ap2;
debug_decl(execl_wrapper, SUDO_DEBUG_EXEC); debug_decl(execl_wrapper, SUDO_DEBUG_EXEC);
if (name == NULL || arg == NULL) {
errno = EINVAL;
debug_return_int(-1);
}
va_copy(ap2, ap); va_copy(ap2, ap);
while (va_arg(ap2, char *) != NULL) while (va_arg(ap2, char *) != NULL)
argc++; argc++;

View File

@@ -297,6 +297,7 @@ send_policy_check_req(int sock, const char *cmnd, char * const argv[],
InterceptRequest msg = INTERCEPT_REQUEST__INIT; InterceptRequest msg = INTERCEPT_REQUEST__INIT;
PolicyCheckRequest req = POLICY_CHECK_REQUEST__INIT; PolicyCheckRequest req = POLICY_CHECK_REQUEST__INIT;
char cwdbuf[PATH_MAX]; char cwdbuf[PATH_MAX];
char *empty[1] = { NULL };
uint8_t *buf = NULL; uint8_t *buf = NULL;
bool ret = false; bool ret = false;
uint32_t msg_len; uint32_t msg_len;
@@ -313,14 +314,14 @@ send_policy_check_req(int sock, const char *cmnd, char * const argv[],
/* Setup policy check request. */ /* Setup policy check request. */
req.intercept_fd = sock; req.intercept_fd = sock;
req.command = (char *)cmnd; req.command = (char *)cmnd;
req.argv = (char **)argv; req.argv = argv ? (char **)argv : empty;
for (len = 0; argv[len] != NULL; len++) req.n_argv = 0;
continue; while (req.argv[req.n_argv] != NULL)
req.n_argv = len; req.n_argv++;
req.envp = (char **)envp; req.envp = envp ? (char **)envp : empty;
for (len = 0; envp[len] != NULL; len++) req.n_envp = 0;
continue; while (req.envp[req.n_envp] != NULL)
req.n_envp = len; req.n_envp++;
if (getcwd(cwdbuf, sizeof(cwdbuf)) != NULL) { if (getcwd(cwdbuf, sizeof(cwdbuf)) != NULL) {
req.cwd = cwdbuf; req.cwd = cwdbuf;
} }
@@ -409,9 +410,11 @@ command_allowed(const char *cmnd, char * const argv[],
if (sudo_debug_needed(SUDO_DEBUG_INFO)) { if (sudo_debug_needed(SUDO_DEBUG_INFO)) {
sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
"req_command: %s", cmnd); "req_command: %s", cmnd);
for (idx = 0; argv[idx] != NULL; idx++) { if (argv != NULL) {
sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, for (idx = 0; argv[idx] != NULL; idx++) {
"req_argv[%zu]: %s", idx, argv[idx]); sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
"req_argv[%zu]: %s", idx, argv[idx]);
}
} }
} }