From d15e117c2eaae7bca2cae16415aa4ffa685fe919 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Wed, 29 Sep 2021 10:25:19 -0600 Subject: [PATCH] find_editor: remove the env_error argument There is no case where we should fail to find an editor just because the values of EDITOR, VISUAL and SUDO_EDITOR are unavailable. Both sudoedit and the "env_editor" sudoers setting are documented as falling back on the hard-coded list of editors in the "editors" sudoers setting. Bug #1000 --- doc/sudoers.man.in | 4 ++-- doc/sudoers.mdoc.in | 4 ++-- plugins/sudoers/editor.c | 15 +++++++-------- plugins/sudoers/regress/editor/check_editor.c | 2 +- plugins/sudoers/sudoers.c | 2 +- plugins/sudoers/sudoers.h | 2 +- plugins/sudoers/visudo.c | 2 +- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/doc/sudoers.man.in b/doc/sudoers.man.in index 15d58bd52..7c58424fd 100644 --- a/doc/sudoers.man.in +++ b/doc/sudoers.man.in @@ -25,7 +25,7 @@ .nr BA @BAMAN@ .nr LC @LCMAN@ .nr PS @PSMAN@ -.TH "SUDOERS" "@mansectform@" "September 17, 2021" "Sudo @PACKAGE_VERSION@" "File Formats Manual" +.TH "SUDOERS" "@mansectform@" "September 29, 2021" "Sudo @PACKAGE_VERSION@" "File Formats Manual" .nh .if n .ad l .SH "NAME" @@ -2471,7 +2471,7 @@ An alternative is to place a colon-separated list of \(lqsafe\(rq editors int the \fIeditor\fR -variable. +setting. \fBvisudo\fR will then only use \fRSUDO_EDITOR\fR, diff --git a/doc/sudoers.mdoc.in b/doc/sudoers.mdoc.in index 70e2731f8..5a6faf1b1 100644 --- a/doc/sudoers.mdoc.in +++ b/doc/sudoers.mdoc.in @@ -24,7 +24,7 @@ .nr BA @BAMAN@ .nr LC @LCMAN@ .nr PS @PSMAN@ -.Dd September 17, 2021 +.Dd September 29, 2021 .Dt SUDOERS @mansectform@ .Os Sudo @PACKAGE_VERSION@ .Sh NAME @@ -2325,7 +2325,7 @@ An alternative is to place a colon-separated list of .Dq safe editors int the .Em editor -variable. +setting. .Nm visudo will then only use .Ev SUDO_EDITOR , diff --git a/plugins/sudoers/editor.c b/plugins/sudoers/editor.c index 99f179e95..db55fc719 100644 --- a/plugins/sudoers/editor.c +++ b/plugins/sudoers/editor.c @@ -205,8 +205,6 @@ oom: /* * Determine which editor to use based on the SUDO_EDITOR, VISUAL and * EDITOR environment variables as well as the editor path in sudoers. - * If env_error is true, an editor environment variable that cannot be - * resolved is an error. * * Returns the path to be executed on success, else NULL. * The caller is responsible for freeing the returned editor path @@ -214,7 +212,7 @@ oom: */ char * find_editor(int nfiles, char **files, int *argc_out, char ***argv_out, - char * const *allowlist, const char **env_editor, bool env_error) + char * const *allowlist, const char **env_editor) { char *ev[3], *editor_path = NULL; unsigned int i; @@ -240,15 +238,16 @@ find_editor(int nfiles, char **files, int *argc_out, char ***argv_out, debug_return_str(NULL); } } + + /* + * If SUDO_EDITOR, VISUAL and EDITOR were either not set or not + * allowed (based on the values of def_editor and def_env_editor), + * choose the first one in def_editor that exists. + */ if (editor_path == NULL) { const char *def_editor_end = def_editor + strlen(def_editor); const char *cp, *ep; - if (env_error && *env_editor != NULL) { - /* User-specified editor could not be found. */ - debug_return_str(NULL); - } - /* def_editor could be a path, split it up, avoiding strtok() */ for (cp = sudo_strsplit(def_editor, def_editor_end, ":", &ep); cp != NULL; cp = sudo_strsplit(NULL, def_editor_end, ":", &ep)) { diff --git a/plugins/sudoers/regress/editor/check_editor.c b/plugins/sudoers/regress/editor/check_editor.c index c2dd40824..97585d66e 100644 --- a/plugins/sudoers/regress/editor/check_editor.c +++ b/plugins/sudoers/regress/editor/check_editor.c @@ -105,7 +105,7 @@ main(int argc, char *argv[]) putenv(data->editor_var); editor_path = find_editor(data->nfiles, data->files, &edit_argc, - &edit_argv, NULL, &env_editor, false); + &edit_argv, NULL, &env_editor); ntests++; if (strcmp(editor_path, data->editor_path) != 0) { sudo_warnx("test %d: editor_path: expected \"%s\", got \"%s\"", diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 51376f970..244eb6a12 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -769,7 +769,7 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], free(safe_cmnd); safe_cmnd = find_editor(NewArgc - 1, NewArgv + 1, &edit_argc, - &edit_argv, NULL, &env_editor, false); + &edit_argv, NULL, &env_editor); if (safe_cmnd == NULL) { if (errno != ENOENT) goto done; diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h index f13d84d17..2993e9379 100644 --- a/plugins/sudoers/sudoers.h +++ b/plugins/sudoers/sudoers.h @@ -445,7 +445,7 @@ extern const char *path_plugin_dir; /* editor.c */ char *find_editor(int nfiles, char **files, int *argc_out, char ***argv_out, - char * const *allowlist, const char **env_editor, bool env_error); + char * const *allowlist, const char **env_editor); /* exptilde.c */ bool expand_tilde(char **path, const char *user); diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c index a2b8b3877..8eb323ba7 100644 --- a/plugins/sudoers/visudo.c +++ b/plugins/sudoers/visudo.c @@ -331,7 +331,7 @@ get_editor(int *editor_argc, char ***editor_argv) } editor_path = find_editor(2, files, editor_argc, editor_argv, allowlist, - &env_editor, true); + &env_editor); if (editor_path == NULL) { if (def_env_editor && env_editor != NULL) { /* We are honoring $EDITOR so this is a fatal error. */