sudo: backport patches for CVE-2021-3156

This security vulnerability is known as Baron Samedit [1] and there is a
research by Qualys [2] and they discovered it. Unfortunately or
fortunately, there isn't present sudoedit on OpenWrt.

Two patches were applied cleanly and the other two required manual
intervention. Those were backported from version 1.9.5p2

[1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-3156
[2] https://blog.qualys.com/vulnerabilities-research/2021/01/26/cve-2021-3156-heap-based-buffer-overflow-in-sudo-baron-samedit

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
This commit is contained in:
Josef Schlehofer 2021-01-27 23:27:57 +01:00 committed by Paul Spooren
parent 4949dcdc50
commit bee91a9d88
2 changed files with 247 additions and 1 deletions

View file

@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
PKG_NAME:=sudo
PKG_VERSION:=1.8.28p1
PKG_RELEASE:=1
PKG_RELEASE:=2
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
PKG_SOURCE_URL:=https://www.sudo.ws/dist

View file

@ -0,0 +1,246 @@
From c21b1fd2a43e67eff4048e624ac77769df951818 Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Sat, 23 Jan 2021 08:43:59 -0700
Subject: [PATCH 1/4] Reset valid_flags to MODE_NONINTERACTIVE for sudoedit.
This is consistent with how the -e option is handled. Also reject -H and -P
flags for sudoedit as was done in sudo 1.7. Found by Qualys, this is part of
the fix for CVE-2021-3156.
---
src/parse_args.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/parse_args.c b/src/parse_args.c
index 260c41f66..883ab80a5 100644
--- a/src/parse_args.c
+++ b/src/parse_args.c
@@ -124,7 +124,10 @@ struct environment {
/*
* Default flags allowed when running a command.
*/
-#define DEFAULT_VALID_FLAGS (MODE_BACKGROUND|MODE_PRESERVE_ENV|MODE_RESET_HOME|MODE_LOGIN_SHELL|MODE_NONINTERACTIVE|MODE_SHELL)
+#define DEFAULT_VALID_FLAGS (MODE_BACKGROUND|MODE_PRESERVE_ENV|MODE_RESET_HOME|MODE_LOGIN_SHELL|MODE_NONINTERACTIVE|MODE_PRESERVE_GROUPS|MODE_SHELL)
+#define EDIT_VALID_FLAGS MODE_NONINTERACTIVE
+#define LIST_VALID_FLAGS (MODE_NONINTERACTIVE|MODE_LONG_LIST)
+#define VALIDATE_VALID_FLAGS MODE_NONINTERACTIVE
/* Option number for the --host long option due to ambiguity of the -h flag. */
#define OPT_HOSTNAME 256
@@ -269,6 +272,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
progname = "sudoedit";
mode = MODE_EDIT;
sudo_settings[ARG_SUDOEDIT].value = "true";
+ valid_flags = EDIT_VALID_FLAGS;
}
/* Load local IP addresses and masks. */
@@ -360,7 +364,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
usage_excl(1);
mode = MODE_EDIT;
sudo_settings[ARG_SUDOEDIT].value = "true";
- valid_flags = MODE_NONINTERACTIVE;
+ valid_flags = EDIT_VALID_FLAGS;
break;
case 'g':
assert(optarg != NULL);
@@ -371,6 +375,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
break;
case 'H':
sudo_settings[ARG_SET_HOME].value = "true";
+ SET(flags, MODE_RESET_HOME);
break;
case 'h':
if (optarg == NULL) {
@@ -421,7 +426,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
usage_excl(1);
}
mode = MODE_LIST;
- valid_flags = MODE_NONINTERACTIVE|MODE_LONG_LIST;
+ valid_flags = LIST_VALID_FLAGS;
break;
case 'n':
SET(flags, MODE_NONINTERACTIVE);
@@ -429,6 +434,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
break;
case 'P':
sudo_settings[ARG_PRESERVE_GROUPS].value = "true";
+ SET(flags, MODE_PRESERVE_GROUPS);
break;
case 'p':
/* An empty prompt is allowed. */
@@ -478,7 +484,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
if (mode && mode != MODE_VALIDATE)
usage_excl(1);
mode = MODE_VALIDATE;
- valid_flags = MODE_NONINTERACTIVE;
+ valid_flags = VALIDATE_VALID_FLAGS;
break;
case 'V':
if (mode && mode != MODE_VERSION)
@@ -505,7 +511,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
if (!mode) {
/* Defer -k mode setting until we know whether it is a flag or not */
if (sudo_settings[ARG_IGNORE_TICKET].value != NULL) {
- if (argc == 0 && !(flags & (MODE_SHELL|MODE_LOGIN_SHELL))) {
+ if (argc == 0 && !ISSET(flags, MODE_SHELL|MODE_LOGIN_SHELL)) {
mode = MODE_INVALIDATE; /* -k by itself */
sudo_settings[ARG_IGNORE_TICKET].value = NULL;
valid_flags = 0;
@@ -568,7 +574,7 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
/*
* For shell mode we need to rewrite argv
*/
- if (ISSET(mode, MODE_RUN) && ISSET(flags, MODE_SHELL)) {
+ if (ISSET(flags, MODE_SHELL|MODE_LOGIN_SHELL) && ISSET(mode, MODE_RUN)) {
char **av, *cmnd = NULL;
int ac = 1;
--
2.25.1
From 75b4169392317cdee95e2ddf1410625e5a1d409b Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Sat, 23 Jan 2021 08:44:00 -0700
Subject: [PATCH 2/4] Don't assume that argv is allocated as a single flat
buffer. While this is how the kernel behaves it is not a portable assumption.
The assumption may also be violated if getopt_long(3) permutes arguments.
Found by Qualys.
---
src/parse_args.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/parse_args.c b/src/parse_args.c
index 883ab80a5..127853c30 100644
--- a/src/parse_args.c
+++ b/src/parse_args.c
@@ -581,16 +581,16 @@ parse_args(int argc, char **argv, int *nargc, char ***nargv,
if (argc != 0) {
/* shell -c "command" */
char *src, *dst;
- size_t cmnd_size = (size_t) (argv[argc - 1] - argv[0]) +
- strlen(argv[argc - 1]) + 1;
+ size_t size = 0;
- cmnd = dst = reallocarray(NULL, cmnd_size, 2);
- if (cmnd == NULL)
+ for (av = argv; *av != NULL; av++)
+ size += strlen(*av) + 1;
+ if (size == 0 || (cmnd = reallocarray(NULL, size, 2)) == NULL)
sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
if (!gc_add(GC_PTR, cmnd))
exit(1);
- for (av = argv; *av != NULL; av++) {
+ for (dst = cmnd, av = argv; *av != NULL; av++) {
for (src = *av; *src != '\0'; src++) {
/* quote potential meta characters */
if (!isalnum((unsigned char)*src) && *src != '_' && *src != '-' && *src != '$')
--
2.25.1
From 58e57a748cbbf5cfb6020d615325257760b4b913 Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Sat, 23 Jan 2021 08:43:59 -0700
Subject: [PATCH 3/4] Fix potential buffer overflow when unescaping backslashes
in user_args.
Also, do not try to unescaping backslashes unless in run mode *and*
we are running the command via a shell.
Found by Qualys, this fixes CVE-2021-3156.
---
plugins/sudoers/sudoers.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c
index 15a002872..1ca779cb3 100644
--- a/plugins/sudoers/sudoers.c
+++ b/plugins/sudoers/sudoers.c
@@ -406,7 +406,7 @@ sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add[],
/* If run as root with SUDO_USER set, set sudo_user.pw to that user. */
/* XXX - causes confusion when root is not listed in sudoers */
- if (sudo_mode & (MODE_RUN | MODE_EDIT) && prev_user != NULL) {
+ if (ISSET(sudo_mode, MODE_RUN|MODE_EDIT) && prev_user != NULL) {
if (user_uid == 0 && strcmp(prev_user, "root") != 0) {
struct passwd *pw;
@@ -786,8 +786,8 @@ set_cmnd(void)
if (user_cmnd == NULL)
user_cmnd = NewArgv[0];
- if (sudo_mode & (MODE_RUN | MODE_EDIT | MODE_CHECK)) {
- if (ISSET(sudo_mode, MODE_RUN | MODE_CHECK)) {
+ if (ISSET(sudo_mode, MODE_RUN|MODE_EDIT|MODE_CHECK)) {
+ if (!ISSET(sudo_mode, MODE_EDIT)) {
if (def_secure_path && !user_is_exempt())
path = def_secure_path;
if (!set_perms(PERM_RUNAS))
@@ -825,7 +825,8 @@ set_cmnd(void)
sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
debug_return_int(-1);
}
- if (ISSET(sudo_mode, MODE_SHELL|MODE_LOGIN_SHELL)) {
+ if (ISSET(sudo_mode, MODE_SHELL|MODE_LOGIN_SHELL) &&
+ ISSET(sudo_mode, MODE_RUN)) {
/*
* When running a command via a shell, the sudo front-end
* escapes potential meta chars. We unescape non-spaces
@@ -833,10 +834,22 @@ set_cmnd(void)
*/
for (to = user_args, av = NewArgv + 1; (from = *av); av++) {
while (*from) {
- if (from[0] == '\\' && !isspace((unsigned char)from[1]))
+ if (from[0] == '\\' && from[1] != '\0' &&
+ !isspace((unsigned char)from[1])) {
from++;
+ }
+ if (size - (to - user_args) < 1) {
+ sudo_warnx(U_("internal error, %s overflow"),
+ __func__);
+ debug_return_int(NOT_FOUND_ERROR);
+ }
*to++ = *from++;
}
+ if (size - (to - user_args) < 1) {
+ sudo_warnx(U_("internal error, %s overflow"),
+ __func__);
+ debug_return_int(NOT_FOUND_ERROR);
+ }
*to++ = ' ';
}
*--to = '\0';
--
2.25.1
From 0754533d2445c93a380c362a185b5464c417455e Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Sat, 23 Jan 2021 08:44:00 -0700
Subject: [PATCH 4/4] Fix the memset offset when converting a v1 timestamp to
TS_LOCKEXCL. We want to zero the struct starting at flags, not type (which
was just set). Found by Qualys.
---
plugins/sudoers/timestamp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/plugins/sudoers/timestamp.c b/plugins/sudoers/timestamp.c
index d315723af..515eadd79 100644
--- a/plugins/sudoers/timestamp.c
+++ b/plugins/sudoers/timestamp.c
@@ -652,8 +652,8 @@ timestamp_lock(void *vcookie, struct passwd *pw)
} else if (entry.type != TS_LOCKEXCL) {
/* Old sudo record, convert it to TS_LOCKEXCL. */
entry.type = TS_LOCKEXCL;
- memset((char *)&entry + offsetof(struct timestamp_entry, type), 0,
- nread - offsetof(struct timestamp_entry, type));
+ memset((char *)&entry + offsetof(struct timestamp_entry, flags), 0,
+ nread - offsetof(struct timestamp_entry, flags));
if (ts_write(cookie->fd, cookie->fname, &entry, 0) == -1)
debug_return_bool(false);
}
--
2.25.1