Avoid calling sudoers_policy_exec_setup() on error.

We only want to pass the execution environment back for commands
that are accepted or rejected.
Also avoid potentially freeing the wrong pointer when garbage
collection is enabled.
This commit is contained in:
Todd C. Miller
2020-02-24 19:59:44 -07:00
parent f40b4c2887
commit f6a264e719
2 changed files with 13 additions and 7 deletions

View File

@@ -816,7 +816,6 @@ sudoers_policy_exec_setup(char *argv[], char *envp[], mode_t cmnd_umask,
#endif /* HAVE_SELINUX */ #endif /* HAVE_SELINUX */
/* Free on exit; they are not available in the close function. */ /* Free on exit; they are not available in the close function. */
sudoers_gc_add(GC_VECTOR, argv);
sudoers_gc_add(GC_VECTOR, envp); sudoers_gc_add(GC_VECTOR, envp);
sudoers_gc_add(GC_VECTOR, command_info); sudoers_gc_add(GC_VECTOR, command_info);

View File

@@ -278,7 +278,6 @@ int
sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
bool verbose, void *closure) bool verbose, void *closure)
{ {
char **edit_argv = NULL;
char *iolog_path = NULL; char *iolog_path = NULL;
mode_t cmnd_umask = ACCESSPERMS; mode_t cmnd_umask = ACCESSPERMS;
struct sudo_nss *nss; struct sudo_nss *nss;
@@ -315,6 +314,7 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
goto done; goto done;
} }
sudoers_gc_add(GC_VECTOR, NewArgv);
NewArgv[0] = user_cmnd; NewArgv[0] = user_cmnd;
NewArgv[1] = NULL; NewArgv[1] = NULL;
} else { } else {
@@ -325,6 +325,7 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
goto done; goto done;
} }
sudoers_gc_add(GC_VECTOR, NewArgv);
NewArgv++; /* reserve an extra slot for --login */ NewArgv++; /* reserve an extra slot for --login */
memcpy(NewArgv, argv, argc * sizeof(char *)); memcpy(NewArgv, argv, argc * sizeof(char *));
NewArgv[NewArgc] = NULL; NewArgv[NewArgc] = NULL;
@@ -332,9 +333,9 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
NewArgv[0] = strdup(runas_pw->pw_shell); NewArgv[0] = strdup(runas_pw->pw_shell);
if (NewArgv[0] == NULL) { if (NewArgv[0] == NULL) {
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
free(NewArgv);
goto done; goto done;
} }
sudoers_gc_add(GC_PTR, NewArgv[0]);
} }
} }
@@ -655,6 +656,7 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
/* Note: must call audit before uid change. */ /* Note: must call audit before uid change. */
if (ISSET(sudo_mode, MODE_EDIT)) { if (ISSET(sudo_mode, MODE_EDIT)) {
char **edit_argv;
int edit_argc; int edit_argc;
const char *env_editor; const char *env_editor;
@@ -670,7 +672,10 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
env_editor ? env_editor : def_editor); env_editor ? env_editor : def_editor);
goto bad; goto bad;
} }
if (audit_success(edit_argc, edit_argv) != 0 && !def_ignore_audit_errors) sudoers_gc_add(GC_VECTOR, edit_argv);
NewArgv = edit_argv;
NewArgc = edit_argc;
if (audit_success(NewArgc, NewArgv) != 0 && !def_ignore_audit_errors)
goto done; goto done;
/* We want to run the editor with the unmodified environment. */ /* We want to run the editor with the unmodified environment. */
@@ -687,9 +692,11 @@ bad:
done: done:
/* Setup execution environment to pass back to front-end. */ /* Setup execution environment to pass back to front-end. */
if (!sudoers_policy_exec_setup(edit_argv ? edit_argv : NewArgv, if (ret != -1) {
env_get(), cmnd_umask, iolog_path, closure)) if (!sudoers_policy_exec_setup(NewArgv, env_get(), cmnd_umask,
iolog_path, closure))
ret = -1; ret = -1;
}
/* Zero out stashed copy of environment, it is owned by the front-end. */ /* Zero out stashed copy of environment, it is owned by the front-end. */
(void)env_init(NULL); (void)env_init(NULL);