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

Don't allow duplicate values for command line options that take an argument.

Browse Source 
Previously, if multiple instances of the same command line option were
specified, the last one would be used.  This meant that, for example,
"sudo -u someuser -u otheruser id" would run the command as "otheruser".
This has the potential to cause problems for programs that run sudo with
a user-specified command that do not use the "--" option to indicate
that no more options should be processed.  While this is a bug in
the calling program, there is little downside to erroring out when
multiple options of the same type are specified on the command line.
Bug #924
This commit is contained in:
Todd C. Miller
2020-05-06 19:33:24 -06:00
parent 24ad424a57
commit 4266279c0c
4 changed files with 47 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
NEWS
View File

@@ -95,6 +95,11 @@ What's new in Sudo 1.9.0
the wrong user name when multiple users in the passwd database
share the the same user-ID. Debian bug #734752.
* Sudo command line options that take a value may only be specified
once. This is to help guard against problems caused by poorly
written scripts that invoke sudo with user-controlled input.
Bug #924.
What's new in Sudo 1.8.31p1
* Sudo once again ignores a failure to restore the RLIMIT_CORE

8
doc/sudo.man.in
View File

@@ -25,7 +25,7 @@
.nr BA @BAMAN@
.nr LC @LCMAN@
.nr PS @PSMAN@
.TH "SUDO" "@mansectsu@" "April 2, 2020" "Sudo @PACKAGE_VERSION@" "System Manager's Manual"
.TH "SUDO" "@mansectsu@" "May 6, 2020" "Sudo @PACKAGE_VERSION@" "System Manager's Manual"
.nh
.if n .ad l
.SH "NAME"
@@ -686,6 +686,12 @@ option indicates that
\fBsudo\fR
should stop processing command line arguments.
.PP
Options that take a value may only be specified once.
This is to help guard against problems caused by poorly written
scripts that invoke
\fBsudo\fR
with user-controlled input.
.PP
Environment variables to be set for the command may also be passed
on the command line in the form of
\fIVAR\fR=\fIvalue\fR,

8
doc/sudo.mdoc.in
View File

@@ -24,7 +24,7 @@
.nr BA @BAMAN@
.nr LC @LCMAN@
.nr PS @PSMAN@
.Dd April 2, 2020
.Dd May 6, 2020
.Dt SUDO @mansectsu@
.Os Sudo @PACKAGE_VERSION@
.Sh NAME
@@ -638,6 +638,12 @@ option indicates that
should stop processing command line arguments.
.El
.Pp
Options that take a value may only be specified once.
This is to help guard against problems caused by poorly written
scripts that invoke
.Nm sudo
with user-controlled input.
.Pp
Environment variables to be set for the command may also be passed
on the command line in the form of
.Ar VAR Ns = Ns Ar value ,

35
src/parse_args.c
View File

@@ -1,7 +1,7 @@
/*
* SPDX-License-Identifier: ISC
*
* Copyright (c) 1993-1996, 1998-2017 Todd C. Miller <Todd.Miller@sudo.ws>
* Copyright (c) 1993-1996, 1998-2020 Todd C. Miller <Todd.Miller@sudo.ws>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -249,8 +249,6 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
int valid_flags = DEFAULT_VALID_FLAGS;
int ch, i;
char *cp;
const char *runas_user = NULL;
const char *runas_group = NULL;
const char *progname;
int proglen;
debug_decl(parse_args, SUDO_DEBUG_ARGS);
@@ -314,6 +312,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
assert(optarg != NULL);
if (*optarg == '\0')
usage();
if (sudo_settings[ARG_BSDAUTH_TYPE].value != NULL)
usage();
sudo_settings[ARG_BSDAUTH_TYPE].value = optarg;
break;
#endif
@@ -329,6 +329,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
sudo_warnx(U_("the argument to -C must be a number greater than or equal to 3"));
usage();
}
if (sudo_settings[ARG_CLOSEFROM].value != NULL)
usage();
sudo_settings[ARG_CLOSEFROM].value = optarg;
break;
#ifdef HAVE_LOGIN_CAP_H
@@ -336,6 +338,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
assert(optarg != NULL);
if (*optarg == '\0')
usage();
if (sudo_settings[ARG_LOGIN_CLASS].value != NULL)
usage();
sudo_settings[ARG_LOGIN_CLASS].value = optarg;
break;
#endif
@@ -352,6 +356,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
sudo_settings[ARG_PRESERVE_ENVIRONMENT].value = "true";
SET(flags, MODE_PRESERVE_ENV);
} else {
if (extra_env.env_len != 0)
usage();
parse_env_list(&extra_env, optarg);
}
break;
@@ -366,7 +372,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
assert(optarg != NULL);
if (*optarg == '\0')
usage();
runas_group = optarg;
if (sudo_settings[ARG_RUNAS_GROUP].value != NULL)
usage();
sudo_settings[ARG_RUNAS_GROUP].value = optarg;
break;
case 'H':
@@ -381,6 +388,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
*/
if (got_host_flag && !is_envar &&
argv[optind] != NULL && argv[optind][0] != '-') {
if (sudo_settings[ARG_REMOTE_HOST].value != NULL)
usage();
sudo_settings[ARG_REMOTE_HOST].value = argv[optind++];
continue;
}
@@ -397,6 +406,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
assert(optarg != NULL);
if (*optarg == '\0')
usage();
if (sudo_settings[ARG_REMOTE_HOST].value != NULL)
usage();
sudo_settings[ARG_REMOTE_HOST].value = optarg;
break;
case 'i':
@@ -433,6 +444,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
case 'p':
/* An empty prompt is allowed. */
assert(optarg != NULL);
if (sudo_settings[ARG_PROMPT].value != NULL)
usage();
sudo_settings[ARG_PROMPT].value = optarg;
break;
#ifdef HAVE_SELINUX
@@ -440,18 +453,24 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
assert(optarg != NULL);
if (*optarg == '\0')
usage();
if (sudo_settings[ARG_SELINUX_ROLE].value != NULL)
usage();
sudo_settings[ARG_SELINUX_ROLE].value = optarg;
break;
case 't':
assert(optarg != NULL);
if (*optarg == '\0')
usage();
if (sudo_settings[ARG_SELINUX_TYPE].value != NULL)
usage();
sudo_settings[ARG_SELINUX_TYPE].value = optarg;
break;
#endif
case 'T':
/* Plugin determines whether empty timeout is allowed. */
assert(optarg != NULL);
if (sudo_settings[ARG_TIMEOUT].value != NULL)
usage();
sudo_settings[ARG_TIMEOUT].value = optarg;
break;
case 'S':
@@ -463,7 +482,7 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
break;
case 'U':
assert(optarg != NULL);
if (*optarg == '\0')
if (list_user != NULL || *optarg == '\0')
usage();
list_user = optarg;
break;
@@ -471,7 +490,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
assert(optarg != NULL);
if (*optarg == '\0')
usage();
runas_user = optarg;
if (sudo_settings[ARG_RUNAS_USER].value != NULL)
usage();
sudo_settings[ARG_RUNAS_USER].value = optarg;
break;
case 'v':
@@ -540,7 +560,8 @@ parse_args(int argc, char **argv, int *old_optind, int *nargc, char ***nargv,
sudo_warnx(U_("you may not specify environment variables in edit mode"));
usage();
}
if ((runas_user != NULL || runas_group != NULL) &&
if ((sudo_settings[ARG_RUNAS_USER].value != NULL ||
sudo_settings[ARG_RUNAS_GROUP].value != NULL) &&
!ISSET(mode, MODE_EDIT | MODE_RUN | MODE_CHECK | MODE_VALIDATE)) {
usage();
}