From 8bc287132be68c205b3770fcad513b0cc191a611 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Thu, 12 Jan 2023 15:55:27 -0700 Subject: [PATCH] sudoedit: do not permit editor arguments to include "--" (CVE-2023-22809) We use "--" to separate the editor and arguments from the files to edit. If the editor arguments include "--", sudo can be tricked into allowing the user to edit a file not permitted by the security policy. Thanks to Matthieu Barjole and Victor Cutillas of Synacktiv (https://synacktiv.com) for finding this bug. Reference:https://github.com/sudo-project/sudo/commit/0274a4f3b403162a37a10f199c989f3727ed3ad4 Conflict:NA --- plugins/sudoers/editor.c | 17 ++++++++++++++++- plugins/sudoers/sudoers.c | 24 +++++++++++++++++------- plugins/sudoers/visudo.c | 8 ++++++-- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/plugins/sudoers/editor.c b/plugins/sudoers/editor.c index 3ff08c1..b65cbd9 100644 --- a/plugins/sudoers/editor.c +++ b/plugins/sudoers/editor.c @@ -50,7 +50,7 @@ resolve_editor(const char *ed, size_t edlen, int nfiles, char **files, const char *cp, *ep, *tmp; const char *edend = ed + edlen; struct stat user_editor_sb; - int nargc; + int nargc = 0; debug_decl(resolve_editor, SUDOERS_DEBUG_UTIL); /* @@ -99,6 +99,21 @@ resolve_editor(const char *ed, size_t edlen, int nfiles, char **files, free(nargv); debug_return_str(NULL); } + + /* + * We use "--" to separate the editor and arguments from the files + * to edit. The editor arguments themselves may not contain "--". + */ + if (strcmp(nargv[nargc], "--") == 0) { + sudo_warnx(U_("ignoring editor: %.*s"), (int)edlen, ed); + sudo_warnx("%s", U_("editor arguments may not contain \"--\"")); + errno = EINVAL; + free(editor_path); + while (nargc--) + free(nargv[nargc]); + free(nargv); + debug_return_str(NULL); + } } if (nfiles != 0) { nargv[nargc++] = "--"; diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index 00a8ce5..14205cb 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -655,21 +655,31 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[], /* Note: must call audit before uid change. */ if (ISSET(sudo_mode, MODE_EDIT)) { + const char *env_editor = NULL; char **edit_argv; int edit_argc; - const char *env_editor; free(safe_cmnd); safe_cmnd = find_editor(NewArgc - 1, NewArgv + 1, &edit_argc, &edit_argv, NULL, &env_editor, false); if (safe_cmnd == NULL) { - if (errno != ENOENT) + switch (errno) { + case ENOENT: + audit_failure(NewArgv, N_("%s: command not found"), + env_editor ? env_editor : def_editor); + sudo_warnx(U_("%s: command not found"), + env_editor ? env_editor : def_editor); + goto bad; + case EINVAL: + if (def_env_editor && env_editor != NULL) { + /* User tried to do something funny with the editor. */ + log_warningx(SLOG_NO_STDERR|SLOG_SEND_MAIL, + "invalid user-specified editor: %s", env_editor); + goto bad; + } + default: goto done; - audit_failure(NewArgv, N_("%s: command not found"), - env_editor ? env_editor : def_editor); - sudo_warnx(U_("%s: command not found"), - env_editor ? env_editor : def_editor); - goto bad; + } } sudoers_gc_add(GC_VECTOR, edit_argv); NewArgv = edit_argv; diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c index 2c64680..b79fc19 100644 --- a/plugins/sudoers/visudo.c +++ b/plugins/sudoers/visudo.c @@ -304,7 +304,7 @@ static char * get_editor(int *editor_argc, char ***editor_argv) { char *editor_path = NULL, **whitelist = NULL; - const char *env_editor; + const char *env_editor = NULL; static char *files[] = { "+1", "sudoers" }; unsigned int whitelist_len = 0; debug_decl(get_editor, SUDOERS_DEBUG_UTIL); @@ -338,7 +338,11 @@ get_editor(int *editor_argc, char ***editor_argv) if (editor_path == NULL) { if (def_env_editor && env_editor != NULL) { /* We are honoring $EDITOR so this is a fatal error. */ - sudo_fatalx(U_("specified editor (%s) doesn't exist"), env_editor); + if (errno == ENOENT) { + sudo_warnx(U_("specified editor (%s) doesn't exist"), + env_editor); + } + exit(EXIT_FAILURE); } sudo_fatalx(U_("no editor found (editor path = %s)"), def_editor); } -- 2.33.0