From 4e48f059e77a788fe7817ea3244baeedde80b0cd Mon Sep 17 00:00:00 2001 From: Chris Hofstaedtler Date: Thu, 13 Jun 2024 03:33:15 +0200 Subject: [PATCH] Drop upstream applied patches --- .../0001-gpasswd-1-Fix-password-leak.patch | 137 ------------------ .../0002-Added-control-character-check.patch | 45 ------ .../patches/0003-Overhaul-valid_field.patch | 61 -------- debian/patches/series | 7 - 4 files changed, 250 deletions(-) delete mode 100644 debian/patches/0001-gpasswd-1-Fix-password-leak.patch delete mode 100644 debian/patches/0002-Added-control-character-check.patch delete mode 100644 debian/patches/0003-Overhaul-valid_field.patch diff --git a/debian/patches/0001-gpasswd-1-Fix-password-leak.patch b/debian/patches/0001-gpasswd-1-Fix-password-leak.patch deleted file mode 100644 index 1596b2d6..00000000 --- a/debian/patches/0001-gpasswd-1-Fix-password-leak.patch +++ /dev/null @@ -1,137 +0,0 @@ -From 65c88a43a23c2391dcc90c0abda3e839e9c57904 Mon Sep 17 00:00:00 2001 -From: Alejandro Colomar -Date: Sat, 10 Jun 2023 16:20:05 +0200 -Subject: [PATCH] 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(+) - ---- a/src/gpasswd.c -+++ b/src/gpasswd.c -@@ -896,6 +896,7 @@ - strzero (cp); - cp = getpass (_("Re-enter new password: ")); - if (NULL == cp) { -+ memzero (pass, sizeof pass); - exit (1); - } - diff --git a/debian/patches/0002-Added-control-character-check.patch b/debian/patches/0002-Added-control-character-check.patch deleted file mode 100644 index 29adce18..00000000 --- a/debian/patches/0002-Added-control-character-check.patch +++ /dev/null @@ -1,45 +0,0 @@ -From e5905c4b84d4fb90aefcd96ee618411ebfac663d 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: [PATCH] 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 640be931..fb51b582 100644 ---- a/lib/fields.c -+++ b/lib/fields.c -@@ -21,9 +21,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) -@@ -45,10 +45,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.34.1 - diff --git a/debian/patches/0003-Overhaul-valid_field.patch b/debian/patches/0003-Overhaul-valid_field.patch deleted file mode 100644 index b7a8428d..00000000 --- a/debian/patches/0003-Overhaul-valid_field.patch +++ /dev/null @@ -1,61 +0,0 @@ -From 2eaea70111f65b16d55998386e4ceb4273c19eb4 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: [PATCH] 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 fb51b582..53929248 100644 ---- a/lib/fields.c -+++ b/lib/fields.c -@@ -37,26 +37,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.34.1 - diff --git a/debian/patches/series b/debian/patches/series index ba058e06..503940b4 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,10 +1,3 @@ -# CVE-2023-4641 -0001-gpasswd-1-Fix-password-leak.patch - -# CVE-2023-29383 -0002-Added-control-character-check.patch -0003-Overhaul-valid_field.patch - # These patches are only for the testsuite: #900_testsuite_groupmems #901_testsuite_gcov