Drop upstream applied patches
This commit is contained in:
Chris Hofstaedtler
@@ -1,137 +0,0 @@
|
||||
From 65c88a43a23c2391dcc90c0abda3e839e9c57904 Mon Sep 17 00:00:00 2001
|
||||
From: Alejandro Colomar <alx@kernel.org>
|
||||
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 <alx@kernel.org>
|
||||
Cc: Serge Hallyn <serge@hallyn.com>
|
||||
Cc: Iker Pedrosa <ipedrosa@redhat.com>
|
||||
Cc: Seth Arnold <seth.arnold@canonical.com>
|
||||
Cc: Christian Brauner <christian@brauner.io>
|
||||
Cc: Balint Reczey <rbalint@debian.org>
|
||||
Cc: Sam James <sam@gentoo.org>
|
||||
Cc: David Runge <dvzrv@archlinux.org>
|
||||
Cc: Andreas Jaeger <aj@suse.de>
|
||||
Cc: <~hallyn/shadow@lists.sr.ht>
|
||||
Signed-off-by: Alejandro Colomar <alx@kernel.org>
|
||||
---
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
61
debian/patches/0003-Overhaul-valid_field.patch
vendored
61
debian/patches/0003-Overhaul-valid_field.patch
vendored
@@ -1,61 +0,0 @@
|
||||
From 2eaea70111f65b16d55998386e4ceb4273c19eb4 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
|
||||
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
|
||||
|
||||
7
debian/patches/series
vendored
7
debian/patches/series
vendored
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user