diff --git a/debian/changelog b/debian/changelog index ad21ceca..0ad067df 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +shadow (1:4.4-4.1+deb9u2) stretch-security; urgency=medium + + * Non-maintainer upload by the ELTS Team. + * CVE-2018-7169: unprivileged user can drop supplementary groups + * CVE-2023-4641: gpasswd password leak + * CVE-2023-29383: chfn missing control character check + + -- Adrian Bunk Sat, 26 Oct 2024 18:55:08 +0300 + shadow (1:4.4-4.1+deb9u1) stretch-security; urgency=high * Non-maintainer upload by the LTS Security Team. diff --git a/debian/patches/0001-newgidmap-enforce-setgroups-deny-if-self-mapping-a-g.patch b/debian/patches/0001-newgidmap-enforce-setgroups-deny-if-self-mapping-a-g.patch new file mode 100644 index 00000000..99712ddb --- /dev/null +++ b/debian/patches/0001-newgidmap-enforce-setgroups-deny-if-self-mapping-a-g.patch @@ -0,0 +1,183 @@ +From f46921b828f06435f8ec1f4ce51f8f622c97f326 Mon Sep 17 00:00:00 2001 +From: Aleksa Sarai +Date: Thu, 15 Feb 2018 23:49:40 +1100 +Subject: newgidmap: enforce setgroups=deny if self-mapping a group + +This is necessary to match the kernel-side policy of "self-mapping in a +user namespace is fine, but you cannot drop groups" -- a policy that was +created in order to stop user namespaces from allowing trivial privilege +escalation by dropping supplementary groups that were "blacklisted" from +certain paths. + +This is the simplest fix for the underlying issue, and effectively makes +it so that unless a user has a valid mapping set in /etc/subgid (which +only administrators can modify) -- and they are currently trying to use +that mapping -- then /proc/$pid/setgroups will be set to deny. This +workaround is only partial, because ideally it should be possible to set +an "allow_setgroups" or "deny_setgroups" flag in /etc/subgid to allow +administrators to further restrict newgidmap(1). + +We also don't write anything in the "allow" case because "allow" is the +default, and users may have already written "deny" even if they +technically are allowed to use setgroups. And we don't write anything if +the setgroups policy is already "deny". + +Ref: https://bugs.launchpad.net/ubuntu/+source/shadow/+bug/1729357 +Fixes: CVE-2018-7169 +Reported-by: Craig Furman +Signed-off-by: Aleksa Sarai +--- + src/newgidmap.c | 89 ++++++++++++++++++++++++++++++++++++++++++++----- + 1 file changed, 80 insertions(+), 9 deletions(-) + +diff --git a/src/newgidmap.c b/src/newgidmap.c +index b1e33513..59a2e75c 100644 +--- a/src/newgidmap.c ++++ b/src/newgidmap.c +@@ -46,32 +46,37 @@ + */ + const char *Prog; + +-static bool verify_range(struct passwd *pw, struct map_range *range) ++ ++static bool verify_range(struct passwd *pw, struct map_range *range, bool *allow_setgroups) + { + /* An empty range is invalid */ + if (range->count == 0) + return false; + +- /* Test /etc/subgid */ +- if (have_sub_gids(pw->pw_name, range->lower, range->count)) ++ /* Test /etc/subgid. If the mapping is valid then we allow setgroups. */ ++ if (have_sub_gids(pw->pw_name, range->lower, range->count)) { ++ *allow_setgroups = true; + return true; ++ } + +- /* Allow a process to map its own gid */ +- if ((range->count == 1) && (pw->pw_gid == range->lower)) ++ /* Allow a process to map its own gid. */ ++ if ((range->count == 1) && (pw->pw_gid == range->lower)) { ++ /* noop -- if setgroups is enabled already we won't disable it. */ + return true; ++ } + + return false; + } + + static void verify_ranges(struct passwd *pw, int ranges, +- struct map_range *mappings) ++ struct map_range *mappings, bool *allow_setgroups) + { + struct map_range *mapping; + int idx; + + mapping = mappings; + for (idx = 0; idx < ranges; idx++, mapping++) { +- if (!verify_range(pw, mapping)) { ++ if (!verify_range(pw, mapping, allow_setgroups)) { + fprintf(stderr, _( "%s: gid range [%lu-%lu) -> [%lu-%lu) not allowed\n"), + Prog, + mapping->upper, +@@ -89,6 +94,70 @@ static void usage(void) + exit(EXIT_FAILURE); + } + ++void write_setgroups(int proc_dir_fd, bool allow_setgroups) ++{ ++ int setgroups_fd; ++ char *policy, policy_buffer[4096]; ++ ++ /* ++ * Default is "deny", and any "allow" will out-rank a "deny". We don't ++ * forcefully write an "allow" here because the process we are writing ++ * mappings for may have already set themselves to "deny" (and "allow" ++ * is the default anyway). So allow_setgroups == true is a noop. ++ */ ++ policy = "deny\n"; ++ if (allow_setgroups) ++ return; ++ ++ setgroups_fd = openat(proc_dir_fd, "setgroups", O_RDWR|O_CLOEXEC); ++ if (setgroups_fd < 0) { ++ /* ++ * If it's an ENOENT then we are on too old a kernel for the setgroups ++ * code to exist. Emit a warning and bail on this. ++ */ ++ if (ENOENT == errno) { ++ fprintf(stderr, _("%s: kernel doesn't support setgroups restrictions\n"), Prog); ++ goto out; ++ } ++ fprintf(stderr, _("%s: couldn't open process setgroups: %s\n"), ++ Prog, ++ strerror(errno)); ++ exit(EXIT_FAILURE); ++ } ++ ++ /* ++ * Check whether the policy is already what we want. /proc/self/setgroups ++ * is write-once, so attempting to write after it's already written to will ++ * fail. ++ */ ++ if (read(setgroups_fd, policy_buffer, sizeof(policy_buffer)) < 0) { ++ fprintf(stderr, _("%s: failed to read setgroups: %s\n"), ++ Prog, ++ strerror(errno)); ++ exit(EXIT_FAILURE); ++ } ++ if (!strncmp(policy_buffer, policy, strlen(policy))) ++ goto out; ++ ++ /* Write the policy. */ ++ if (lseek(setgroups_fd, 0, SEEK_SET) < 0) { ++ fprintf(stderr, _("%s: failed to seek setgroups: %s\n"), ++ Prog, ++ strerror(errno)); ++ exit(EXIT_FAILURE); ++ } ++ if (dprintf(setgroups_fd, "%s", policy) < 0) { ++ fprintf(stderr, _("%s: failed to setgroups %s policy: %s\n"), ++ Prog, ++ policy, ++ strerror(errno)); ++ exit(EXIT_FAILURE); ++ } ++ ++out: ++ close(setgroups_fd); ++} ++ + /* + * newgidmap - Set the gid_map for the specified process + */ +@@ -103,6 +172,7 @@ int main(int argc, char **argv) + struct stat st; + struct passwd *pw; + int written; ++ bool allow_setgroups = false; + + Prog = Basename (argv[0]); + +@@ -145,7 +215,7 @@ int main(int argc, char **argv) + (unsigned long) getuid ())); + return EXIT_FAILURE; + } +- ++ + /* Get the effective uid and effective gid of the target process */ + if (fstat(proc_dir_fd, &st) < 0) { + fprintf(stderr, _("%s: Could not stat directory for target %u\n"), +@@ -177,8 +247,9 @@ int main(int argc, char **argv) + if (!mappings) + usage(); + +- verify_ranges(pw, ranges, mappings); ++ verify_ranges(pw, ranges, mappings, &allow_setgroups); + ++ write_setgroups(proc_dir_fd, allow_setgroups); + write_mapping(proc_dir_fd, ranges, mappings, "gid_map"); + sub_gid_close(); + +-- +2.30.2 + diff --git a/debian/patches/0002-gpasswd-1-Fix-password-leak.patch b/debian/patches/0002-gpasswd-1-Fix-password-leak.patch new file mode 100644 index 00000000..f9275ed2 --- /dev/null +++ b/debian/patches/0002-gpasswd-1-Fix-password-leak.patch @@ -0,0 +1,142 @@ +From c64784990ca4de6e998f67796faa7bafc15dab00 Mon Sep 17 00:00:00 2001 +From: Alejandro Colomar +Date: Sat, 10 Jun 2023 16:20:05 +0200 +Subject: gpasswd(1): Fix password leak + +How to trigger this password leak? +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When gpasswd(1) asks for the new password, it asks twice (as is usual +for confirming the new password). Each of those 2 password prompts +uses agetpass() to get the password. If the second agetpass() fails, +the first password, which has been copied into the 'static' buffer +'pass' via STRFCPY(), wasn't being zeroed. + +agetpass() is defined in <./libmisc/agetpass.c> (around line 91), and +can fail for any of the following reasons: + +- malloc(3) or readpassphrase(3) failure. + + These are going to be difficult to trigger. Maybe getting the system + to the limits of memory utilization at that exact point, so that the + next malloc(3) gets ENOMEM, and possibly even the OOM is triggered. + About readpassphrase(3), ENFILE and EINTR seem the only plausible + ones, and EINTR probably requires privilege or being the same user; + but I wouldn't discard ENFILE so easily, if a process starts opening + files. + +- The password is longer than PASS_MAX. + + The is plausible with physical access. However, at that point, a + keylogger will be a much simpler attack. + +And, the attacker must be able to know when the second password is being +introduced, which is not going to be easy. + +How to read the password after the leak? +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Provoking the leak yourself at the right point by entering a very long +password is easy, and inspecting the process stack at that point should +be doable. Try to find some consistent patterns. + +Then, search for those patterns in free memory, right after the victim +leaks their password. + +Once you get the leak, a program should read all the free memory +searching for patterns that gpasswd(1) leaves nearby the leaked +password. + +On 6/10/23 03:14, Seth Arnold wrote: +> An attacker process wouldn't be able to use malloc(3) for this task. +> There's a handful of tools available for userspace to allocate memory: +> +> - brk / sbrk +> - mmap MAP_ANONYMOUS +> - mmap /dev/zero +> - mmap some other file +> - shm_open +> - shmget +> +> Most of these return only pages of zeros to a process. Using mmap of an +> existing file, you can get some of the contents of the file demand-loaded +> into the memory space on the first use. +> +> The MAP_UNINITIALIZED flag only works if the kernel was compiled with +> CONFIG_MMAP_ALLOW_UNINITIALIZED. This is rare. +> +> malloc(3) doesn't zero memory, to our collective frustration, but all the +> garbage in the allocations is from previous allocations in the current +> process. It isn't leftover from other processes. +> +> The avenues available for reading the memory: +> - /dev/mem and /dev/kmem (requires root, not available with Secure Boot) +> - /proc/pid/mem (requires ptrace privileges, mediated by YAMA) +> - ptrace (requires ptrace privileges, mediated by YAMA) +> - causing memory to be swapped to disk, and then inspecting the swap +> +> These all require a certain amount of privileges. + +How to fix it? +~~~~~~~~~~~~~~ + +memzero(), which internally calls explicit_bzero(3), or whatever +alternative the system provides with a slightly different name, will +make sure that the buffer is zeroed in memory, and optimizations are not +allowed to impede this zeroing. + +This is not really 100% effective, since compilers may place copies of +the string somewhere hidden in the stack. Those copies won't get zeroed +by explicit_bzero(3). However, that's arguably a compiler bug, since +compilers should make everything possible to avoid optimizing strings +that are later passed to explicit_bzero(3). But we all know that +sometimes it's impossible to have perfect knowledge in the compiler, so +this is plausible. Nevertheless, there's nothing we can do against such +issues, except minimizing the time such passwords are stored in plain +text. + +Security concerns +~~~~~~~~~~~~~~~~~ + +We believe this isn't easy to exploit. Nevertheless, and since the fix +is trivial, this fix should probably be applied soon, and backported to +all supported distributions, to prevent someone else having more +imagination than us to find a way. + +Affected versions +~~~~~~~~~~~~~~~~~ + +All. Bug introduced in shadow 19990709. That's the second commit in +the git history. + +Fixes: 45c6603cc86c ("[svn-upgrade] Integrating new upstream version, shadow (19990709)") +Reported-by: Alejandro Colomar +Cc: Serge Hallyn +Cc: Iker Pedrosa +Cc: Seth Arnold +Cc: Christian Brauner +Cc: Balint Reczey +Cc: Sam James +Cc: David Runge +Cc: Andreas Jaeger +Cc: <~hallyn/shadow@lists.sr.ht> +Signed-off-by: Alejandro Colomar +--- + src/gpasswd.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/gpasswd.c b/src/gpasswd.c +index c4a492b1..cbbd8068 100644 +--- a/src/gpasswd.c ++++ b/src/gpasswd.c +@@ -917,6 +917,7 @@ static void change_passwd (struct group *gr) + strzero (cp); + cp = getpass (_("Re-enter new password: ")); + if (NULL == cp) { ++ memzero (pass, sizeof pass); + exit (1); + } + +-- +2.30.2 + diff --git a/debian/patches/0003-Added-control-character-check.patch b/debian/patches/0003-Added-control-character-check.patch new file mode 100644 index 00000000..ebb6158d --- /dev/null +++ b/debian/patches/0003-Added-control-character-check.patch @@ -0,0 +1,45 @@ +From d6f0f7cd86b189cf3bbd49e404864cb599e10244 Mon Sep 17 00:00:00 2001 +From: tomspiderlabs <128755403+tomspiderlabs@users.noreply.github.com> +Date: Thu, 23 Mar 2023 23:39:38 +0000 +Subject: Added control character check + +Added control character check, returning -1 (to "err") if control characters are present. +--- + lib/fields.c | 11 +++++++---- + 1 file changed, 7 insertions(+), 4 deletions(-) + +diff --git a/lib/fields.c b/lib/fields.c +index 649fae17..b8f13ba7 100644 +--- a/lib/fields.c ++++ b/lib/fields.c +@@ -44,9 +44,9 @@ + * + * The supplied field is scanned for non-printable and other illegal + * characters. +- * + -1 is returned if an illegal character is present. +- * + 1 is returned if no illegal characters are present, but the field +- * contains a non-printable character. ++ * + -1 is returned if an illegal or control character is present. ++ * + 1 is returned if no illegal or control characters are present, ++ * but the field contains a non-printable character. + * + 0 is returned otherwise. + */ + int valid_field (const char *field, const char *illegal) +@@ -68,10 +68,13 @@ int valid_field (const char *field, const char *illegal) + } + + if (0 == err) { +- /* Search if there are some non-printable characters */ ++ /* Search if there are non-printable or control characters */ + for (cp = field; '\0' != *cp; cp++) { + if (!isprint (*cp)) { + err = 1; ++ } ++ if (!iscntrl (*cp)) { ++ err = -1; + break; + } + } +-- +2.30.2 + diff --git a/debian/patches/0004-Overhaul-valid_field.patch b/debian/patches/0004-Overhaul-valid_field.patch new file mode 100644 index 00000000..fda85804 --- /dev/null +++ b/debian/patches/0004-Overhaul-valid_field.patch @@ -0,0 +1,61 @@ +From aad293ef78b1657978adb2049974805bf20af5bb Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= +Date: Fri, 31 Mar 2023 14:46:50 +0200 +Subject: Overhaul valid_field() + +e5905c4b ("Added control character check") introduced checking for +control characters but had the logic inverted, so it rejects all +characters that are not control ones. + +Cast the character to `unsigned char` before passing to the character +checking functions to avoid UB. + +Use strpbrk(3) for the illegal character test and return early. +--- + lib/fields.c | 24 ++++++++++-------------- + 1 file changed, 10 insertions(+), 14 deletions(-) + +diff --git a/lib/fields.c b/lib/fields.c +index b8f13ba7..191257e8 100644 +--- a/lib/fields.c ++++ b/lib/fields.c +@@ -60,26 +60,22 @@ int valid_field (const char *field, const char *illegal) + + /* For each character of field, search if it appears in the list + * of illegal characters. */ ++ if (illegal && NULL != strpbrk (field, illegal)) { ++ return -1; ++ } ++ ++ /* Search if there are non-printable or control characters */ + for (cp = field; '\0' != *cp; cp++) { +- if (strchr (illegal, *cp) != NULL) { ++ unsigned char c = *cp; ++ if (!isprint (c)) { ++ err = 1; ++ } ++ if (iscntrl (c)) { + err = -1; + break; + } + } + +- if (0 == err) { +- /* Search if there are non-printable or control characters */ +- for (cp = field; '\0' != *cp; cp++) { +- if (!isprint (*cp)) { +- err = 1; +- } +- if (!iscntrl (*cp)) { +- err = -1; +- break; +- } +- } +- } +- + return err; + } + +-- +2.30.2 + diff --git a/debian/patches/series b/debian/patches/series index 1cdf3ac0..0d041eec 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -30,3 +30,7 @@ 1010_vietnamese_translation CVE-2017-12424.patch +0001-newgidmap-enforce-setgroups-deny-if-self-mapping-a-g.patch +0002-gpasswd-1-Fix-password-leak.patch +0003-Added-control-character-check.patch +0004-Overhaul-valid_field.patch