From ae3a098d2f0988b63a780523a006a3db8e8b3872 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Tue, 16 Feb 2021 09:32:34 -0700 Subject: [PATCH] Always dynamically allocate user_cmnd, it is freed in sudo_user_free(). Instead of setting user_cmnd in the policy functions, always set argv. Calling sudoers_policy_main() with argc of 0 is no longer allowed. --- plugins/sudoers/policy.c | 20 ++++--- plugins/sudoers/regress/fuzz/fuzz_policy.c | 8 +-- plugins/sudoers/sudoers.c | 67 +++++++++++----------- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/plugins/sudoers/policy.c b/plugins/sudoers/policy.c index e3e0060c0..79572f0dc 100644 --- a/plugins/sudoers/policy.c +++ b/plugins/sudoers/policy.c @@ -348,6 +348,7 @@ sudoers_policy_deserialize_info(void *v) } /* Sudo front-end should restrict mode flags for sudoedit. */ + /* XXX - also restrict pseudo-commands */ if (ISSET(flags, MODE_EDIT) && (flags & edit_mask) != flags) { sudo_warnx(U_("invalid mode flags from sudo front end: 0x%x"), flags); goto bad; @@ -976,6 +977,8 @@ sudoers_policy_close(int exit_status, int error_code) free(audit_msg); audit_msg = NULL; + /* XXX - leaks NewArgv */ + /* sudoers_debug_deregister() calls sudo_debug_exit() for us. */ sudoers_debug_deregister(); } @@ -1045,13 +1048,13 @@ sudoers_policy_check(int argc, char * const argv[], char *env_add[], static int sudoers_policy_validate(const char **errstr) { + char *argv[] = { "validate", NULL }; + const int argc = 1; int ret; debug_decl(sudoers_policy_validate, SUDOERS_DEBUG_PLUGIN); - user_cmnd = "validate"; SET(sudo_mode, MODE_VALIDATE); - - ret = sudoers_policy_main(0, NULL, I_VERIFYPW, NULL, false, NULL); + ret = sudoers_policy_main(argc, argv, I_VERIFYPW, NULL, false, NULL); /* The audit functions set audit_msg on failure. */ if (ret != 1 && audit_msg != NULL) { @@ -1075,14 +1078,17 @@ static int sudoers_policy_list(int argc, char * const argv[], int verbose, const char *list_user, const char **errstr) { + char *list_argv[] = { "list", NULL }; int ret; debug_decl(sudoers_policy_list, SUDOERS_DEBUG_PLUGIN); - user_cmnd = "list"; - if (argc) - SET(sudo_mode, MODE_CHECK); - else + if (argc == 0) { SET(sudo_mode, MODE_LIST); + argc = 1; + argv = list_argv; + } else { + SET(sudo_mode, MODE_CHECK); + } if (list_user) { list_pw = sudo_getpwnam(list_user); if (list_pw == NULL) { diff --git a/plugins/sudoers/regress/fuzz/fuzz_policy.c b/plugins/sudoers/regress/fuzz/fuzz_policy.c index 37f1df618..7f8b80dc2 100644 --- a/plugins/sudoers/regress/fuzz/fuzz_policy.c +++ b/plugins/sudoers/regress/fuzz/fuzz_policy.c @@ -302,7 +302,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) continue; } - /* First argv entry is the command, the rest are args. */ + /* Additional environment variables to add. */ if (strncmp(line, "env=", sizeof("env=") - 1) == 0) { push(&env_add, line); continue; @@ -340,10 +340,6 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) break; } - /* Avoid double free of user_cmnd, it will be freed as part of argv. */ - if (argv.len != 0 && user_cmnd == argv.entries[0]) - user_cmnd = NULL; - /* Free resources. */ sudoers_policy.close(0, 0); @@ -639,7 +635,7 @@ find_path(const char *infile, char **outfile, struct stat *sbp, if (asprintf(outfile, "/usr/bin/%s", infile) == -1) *outfile = NULL; } - return *outfile ? FOUND : NOT_FOUND; + return *outfile ? FOUND : NOT_FOUND_ERROR; } /* STUB */ diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 8d1a68fe6..8543ab75d 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -341,6 +341,11 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], sudo_warn_set_locale_func(sudoers_warn_setlocale); + if (argc == 0) { + sudo_warnx("%s", U_("no command specified")); + debug_return_int(-1); + } + unlimit_nproc(); /* Is root even allowed to run sudo? */ @@ -360,38 +365,26 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], /* * Make a local copy of argc/argv, with special handling - * for pseudo-commands and the '-i' option. + * for the '-i' option. */ - if (argc == 0) { - NewArgc = 1; - NewArgv = reallocarray(NULL, NewArgc + 1, sizeof(char *)); - if (NewArgv == NULL) { + /* Must leave an extra slot before NewArgv for bash's --login */ + NewArgc = argc; + NewArgv = reallocarray(NULL, NewArgc + 2, sizeof(char *)); + if (NewArgv == NULL) { + sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); + goto done; + } + sudoers_gc_add(GC_PTR, NewArgv); + NewArgv++; /* reserve an extra slot for --login */ + memcpy(NewArgv, argv, argc * sizeof(char *)); + NewArgv[NewArgc] = NULL; + if (ISSET(sudo_mode, MODE_LOGIN_SHELL) && runas_pw != NULL) { + NewArgv[0] = strdup(runas_pw->pw_shell); + if (NewArgv[0] == NULL) { sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); goto done; } - sudoers_gc_add(GC_PTR, NewArgv); - NewArgv[0] = user_cmnd; - NewArgv[1] = NULL; - } else { - /* Must leave an extra slot before NewArgv for bash's --login */ - NewArgc = argc; - NewArgv = reallocarray(NULL, NewArgc + 2, sizeof(char *)); - if (NewArgv == NULL) { - sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); - goto done; - } - sudoers_gc_add(GC_PTR, NewArgv); - NewArgv++; /* reserve an extra slot for --login */ - memcpy(NewArgv, argv, argc * sizeof(char *)); - NewArgv[NewArgc] = NULL; - if (ISSET(sudo_mode, MODE_LOGIN_SHELL) && runas_pw != NULL) { - NewArgv[0] = strdup(runas_pw->pw_shell); - if (NewArgv[0] == NULL) { - sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); - goto done; - } - sudoers_gc_add(GC_PTR, NewArgv[0]); - } + sudoers_gc_add(GC_PTR, NewArgv[0]); } /* If given the -P option, set the "preserve_groups" flag. */ @@ -930,10 +923,6 @@ set_cmnd(void) debug_return_int(NOT_FOUND_ERROR); } - /* Default value for cmnd, overridden by set_cmnd_path() below. */ - if (user_cmnd == NULL) - user_cmnd = NewArgv[0]; - if (ISSET(sudo_mode, MODE_RUN|MODE_EDIT|MODE_CHECK)) { if (!ISSET(sudo_mode, MODE_EDIT)) { const char *runchroot = user_runchroot; @@ -968,14 +957,26 @@ set_cmnd(void) debug_return_int(NOT_FOUND_ERROR); } } + if (user_cmnd == NULL) { + user_cmnd = strdup(NewArgv[0]); + if (user_cmnd == NULL) + debug_return_int(NOT_FOUND_ERROR); + } user_base = sudo_basename(user_cmnd); /* Convert "sudo sudoedit" -> "sudoedit" */ if (ISSET(sudo_mode, MODE_RUN) && strcmp(user_base, "sudoedit") == 0) { + char *new_cmnd; + CLR(sudo_mode, MODE_RUN); SET(sudo_mode, MODE_EDIT); sudo_warnx("%s", U_("sudoedit doesn't need to be run via sudo")); - user_base = user_cmnd = "sudoedit"; + if ((new_cmnd = strdup("sudoedit")) == NULL) { + sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); + debug_return_int(NOT_FOUND_ERROR); + } + free(user_cmnd); + user_base = user_cmnd = new_cmnd; } TAILQ_FOREACH(nss, snl, entries) {