isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

sudoedit: do not permit editor arguments to include "--" (CVE-2023-22809)

Browse Source 
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.
This commit is contained in:
Todd C. Miller
2023-01-12 15:55:27 -07:00
parent a960d2c45f
commit 0274a4f3b4
3 changed files with 38 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
plugins/sudoers/editor.c
View File

@@ -131,7 +131,7 @@ resolve_editor(const char *ed, size_t edlen, int nfiles, char * const *files,
const char *tmp, *cp, *ep = NULL;
const char *edend = ed + edlen;
struct stat user_editor_sb;
int nargc;
int nargc = 0;
debug_decl(resolve_editor, SUDOERS_DEBUG_UTIL);
/*
@@ -149,10 +149,7 @@ resolve_editor(const char *ed, size_t edlen, int nfiles, char * const *files,
/* If we can't find the editor in the user's PATH, give up. */
if (find_path(editor, &editor_path, &user_editor_sb, getenv("PATH"), NULL,
0, allowlist) != FOUND) {
sudoers_gc_remove(GC_PTR, editor);
free(editor);
errno = ENOENT;
debug_return_str(NULL);
goto bad;
}
/* Count rest of arguments and allocate editor argv. */
@@ -173,6 +170,17 @@ resolve_editor(const char *ed, size_t edlen, int nfiles, char * const *files,
nargv[nargc] = copy_arg(cp, ep - cp);
if (nargv[nargc] == NULL)
goto oom;
/*
* 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;
goto bad;
}
}
if (nfiles != 0) {
nargv[nargc++] = (char *)"--";
@@ -186,6 +194,7 @@ resolve_editor(const char *ed, size_t edlen, int nfiles, char * const *files,
debug_return_str(editor_path);
oom:
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
bad:
sudoers_gc_remove(GC_PTR, editor);
free(editor);
free(editor_path);

17
plugins/sudoers/sudoers.c
View File

@@ -802,21 +802,32 @@ 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);
if (safe_cmnd == NULL) {
if (errno != ENOENT)
goto done;
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_AUDIT|SLOG_SEND_MAIL,
"invalid user-specified editor: %s", env_editor);
goto bad;
}
FALLTHROUGH;
default:
goto done;
}
}
/* find_editor() already g/c'd edit_argv[] */
if (NewArgv != saved_argv) {

8
plugins/sudoers/visudo.c
View File

@@ -365,7 +365,7 @@ static char *
get_editor(int *editor_argc, char ***editor_argv)
{
char *editor_path = NULL, **allowlist = NULL;
const char *env_editor;
const char *env_editor = NULL;
static const char *files[] = { "+1", "sudoers" };
unsigned int allowlist_len = 0;
debug_decl(get_editor, SUDOERS_DEBUG_UTIL);
@@ -399,7 +399,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);
}