1. photon
  2. Commit 0f4d3630cad1525fb6c2d841779128ff9bd3f83f
Browse code

shadow: patched CVE-2023-29383 and CVE-2023-4641

Change-Id: Ib4fcbffccee60f03ed291860a5a4f090e4ba5ec9
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/c/photon/+/22903
Reviewed-by: Shreenidhi Shedi <shreenidhi.shedi@broadcom.com>
Tested-by: gerrit-photon <photon-checkins@vmware.com>

Srish Srinivasan authored on 2023/12/21 15:07:14
Showing 4 changed files
SPECS/shadow/CVE-2023-29383.1.patch
History View file @ 0f4d363
1 1 
new file mode 100644
... ... 
@@ -0,0 +1,57 @@
0 
+From 2eaea70111f65b16d55998386e4ceb4273c19eb4 Mon Sep 17 00:00:00 2001
1 
+From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
2 
+Date: Fri, 31 Mar 2023 14:46:50 +0200
3 
+Subject: [PATCH] Overhaul valid_field()
4 
+
5 
+e5905c4b ("Added control character check") introduced checking for
6 
+control characters but had the logic inverted, so it rejects all
7 
+characters that are not control ones.
8 
+
9 
+Cast the character to `unsigned char` before passing to the character
10 
+checking functions to avoid UB.
11 
+
12 
+Use strpbrk(3) for the illegal character test and return early.
13 
+---
14 
+ lib/fields.c | 24 ++++++++++--------------
15 
+ 1 file changed, 10 insertions(+), 14 deletions(-)
16 
+
17 
+diff --git a/lib/fields.c b/lib/fields.c
18 
+index fb51b5829..539292485 100644
19 
+--- a/lib/fields.c
20 
+@@ -37,26 +37,22 @@ int valid_field (const char *field, const char *illegal)
21 
+
22 
+ 	/* For each character of field, search if it appears in the list
23 
+ 	 * of illegal characters. */
24 
++	if (illegal && NULL != strpbrk (field, illegal)) {
25 
++		return -1;
26 
++	}
27 
++
28 
++	/* Search if there are non-printable or control characters */
29 
+ 	for (cp = field; '\0' != *cp; cp++) {
30 
+-		if (strchr (illegal, *cp) != NULL) {
31 
++		unsigned char c = *cp;
32 
++		if (!isprint (c)) {
33 
++			err = 1;
34 
++		}
35 
++		if (iscntrl (c)) {
36 
+ 			err = -1;
37 
+ 			break;
38 
+ 		}
39 
+ 	}
40 
+
41 
+-	if (0 == err) {
42 
+-		/* Search if there are non-printable or control characters */
43 
+-		for (cp = field; '\0' != *cp; cp++) {
44 
+-			if (!isprint (*cp)) {
45 
+-				err = 1;
46 
+-			}
47 
+-			if (!iscntrl (*cp)) {
48 
+-				err = -1;
49 
+-				break;
50 
+-			}
51 
+-		}
52 
+-	}
53 
+-
54 
+ 	return err;
55 
+ }
SPECS/shadow/CVE-2023-29383.patch
History View file @ 0f4d363
0 56 
new file mode 100644
... ... 
@@ -0,0 +1,43 @@
0 
+From e5905c4b84d4fb90aefcd96ee618411ebfac663d Mon Sep 17 00:00:00 2001
1 
+From: tomspiderlabs <128755403+tomspiderlabs@users.noreply.github.com>
2 
+Date: Thu, 23 Mar 2023 23:39:38 +0000
3 
+Subject: [PATCH] Added control character check
4 
+
5 
+Added control character check, returning -1 (to "err") if control characters are present.
6 
+---
7 
+ lib/fields.c | 11 +++++++----
8 
+ 1 file changed, 7 insertions(+), 4 deletions(-)
9 
+
10 
+diff --git a/lib/fields.c b/lib/fields.c
11 
+index 640be931f..fb51b5829 100644
12 
+--- a/lib/fields.c
13 
+@@ -21,9 +21,9 @@
14 
+  *
15 
+  * The supplied field is scanned for non-printable and other illegal
16 
+  * characters.
17 
+- *  + -1 is returned if an illegal character is present.
18 
+- *  +  1 is returned if no illegal characters are present, but the field
19 
+- *       contains a non-printable character.
20 
++ *  + -1 is returned if an illegal or control character is present.
21 
++ *  +  1 is returned if no illegal or control characters are present,
22 
++ *       but the field contains a non-printable character.
23 
+  *  +  0 is returned otherwise.
24 
+  */
25 
+ int valid_field (const char *field, const char *illegal)
26 
+@@ -45,10 +45,13 @@ int valid_field (const char *field, const char *illegal)
27 
+ 	}
28 
+
29 
+ 	if (0 == err) {
30 
+-		/* Search if there are some non-printable characters */
31 
++		/* Search if there are non-printable or control characters */
32 
+ 		for (cp = field; '\0' != *cp; cp++) {
33 
+ 			if (!isprint (*cp)) {
34 
+ 				err = 1;
35 
++			}
36 
++			if (!iscntrl (*cp)) {
37 
++				err = -1;
38 
+ 				break;
39 
+ 			}
40 
+ 		}
41 
+
SPECS/shadow/CVE-2023-4641.patch
History View file @ 0f4d363
0 42 
new file mode 100644
... ... 
@@ -0,0 +1,139 @@
0 
+From 65c88a43a23c2391dcc90c0abda3e839e9c57904 Mon Sep 17 00:00:00 2001
1 
+From: Alejandro Colomar <alx@kernel.org>
2 
+Date: Sat, 10 Jun 2023 16:20:05 +0200
3 
+Subject: [PATCH] gpasswd(1): Fix password leak
4 
+
5 
+How to trigger this password leak?
6 
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7 
+
8 
+When gpasswd(1) asks for the new password, it asks twice (as is usual
9 
+for confirming the new password).  Each of those 2 password prompts
10 
+uses agetpass() to get the password.  If the second agetpass() fails,
11 
+the first password, which has been copied into the 'static' buffer
12 
+'pass' via STRFCPY(), wasn't being zeroed.
13 
+
14 
+agetpass() is defined in <./libmisc/agetpass.c> (around line 91), and
15 
+can fail for any of the following reasons:
16 
+
17 
+-  malloc(3) or readpassphrase(3) failure.
18 
+
19 
+   These are going to be difficult to trigger.  Maybe getting the system
20 
+   to the limits of memory utilization at that exact point, so that the
21 
+   next malloc(3) gets ENOMEM, and possibly even the OOM is triggered.
22 
+   About readpassphrase(3), ENFILE and EINTR seem the only plausible
23 
+   ones, and EINTR probably requires privilege or being the same user;
24 
+   but I wouldn't discard ENFILE so easily, if a process starts opening
25 
+   files.
26 
+
27 
+-  The password is longer than PASS_MAX.
28 
+
29 
+   The is plausible with physical access.  However, at that point, a
30 
+   keylogger will be a much simpler attack.
31 
+
32 
+And, the attacker must be able to know when the second password is being
33 
+introduced, which is not going to be easy.
34 
+
35 
+How to read the password after the leak?
36 
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
37 
+
38 
+Provoking the leak yourself at the right point by entering a very long
39 
+password is easy, and inspecting the process stack at that point should
40 
+be doable.  Try to find some consistent patterns.
41 
+
42 
+Then, search for those patterns in free memory, right after the victim
43 
+leaks their password.
44 
+
45 
+Once you get the leak, a program should read all the free memory
46 
+searching for patterns that gpasswd(1) leaves nearby the leaked
47 
+password.
48 
+
49 
+On 6/10/23 03:14, Seth Arnold wrote:
50 
+> An attacker process wouldn't be able to use malloc(3) for this task.
51 
+> There's a handful of tools available for userspace to allocate memory:
52 
+>
53 
+> -  brk / sbrk
54 
+> -  mmap MAP_ANONYMOUS
55 
+> -  mmap /dev/zero
56 
+> -  mmap some other file
57 
+> -  shm_open
58 
+> -  shmget
59 
+>
60 
+> Most of these return only pages of zeros to a process.  Using mmap of an
61 
+> existing file, you can get some of the contents of the file demand-loaded
62 
+> into the memory space on the first use.
63 
+>
64 
+> The MAP_UNINITIALIZED flag only works if the kernel was compiled with
65 
+> CONFIG_MMAP_ALLOW_UNINITIALIZED.  This is rare.
66 
+>
67 
+> malloc(3) doesn't zero memory, to our collective frustration, but all the
68 
+> garbage in the allocations is from previous allocations in the current
69 
+> process.  It isn't leftover from other processes.
70 
+>
71 
+> The avenues available for reading the memory:
72 
+> -  /dev/mem and /dev/kmem (requires root, not available with Secure Boot)
73 
+> -  /proc/pid/mem (requires ptrace privileges, mediated by YAMA)
74 
+> -  ptrace (requires ptrace privileges, mediated by YAMA)
75 
+> -  causing memory to be swapped to disk, and then inspecting the swap
76 
+>
77 
+> These all require a certain amount of privileges.
78 
+
79 
+How to fix it?
80 
+~~~~~~~~~~~~
81 
+
82 
+memzero(), which internally calls explicit_bzero(3), or whatever
83 
+alternative the system provides with a slightly different name, will
84 
+make sure that the buffer is zeroed in memory, and optimizations are not
85 
+allowed to impede this zeroing.
86 
+
87 
+This is not really 100% effective, since compilers may place copies of
88 
+the string somewhere hidden in the stack.  Those copies won't get zeroed
89 
+by explicit_bzero(3).  However, that's arguably a compiler bug, since
90 
+compilers should make everything possible to avoid optimizing strings
91 
+that are later passed to explicit_bzero(3).  But we all know that
92 
+sometimes it's impossible to have perfect knowledge in the compiler, so
93 
+this is plausible.  Nevertheless, there's nothing we can do against such
94 
+issues, except minimizing the time such passwords are stored in plain
95 
+text.
96 
+
97 
+Security concerns
98 
+~~~~~~~~~~~~~~~
99 
+
100 
+We believe this isn't easy to exploit.  Nevertheless, and since the fix
101 
+is trivial, this fix should probably be applied soon, and backported to
102 
+all supported distributions, to prevent someone else having more
103 
+imagination than us to find a way.
104 
+
105 
+Affected versions
106 
+~~~~~~~~~~~~~~~
107 
+
108 
+All.  Bug introduced in shadow 19990709.  That's the second commit in
109 
+the git history.
110 
+
111 
+Fixes: 45c6603cc86c ("[svn-upgrade] Integrating new upstream version, shadow (19990709)")
112 
+Reported-by: Alejandro Colomar <alx@kernel.org>
113 
+Cc: Serge Hallyn <serge@hallyn.com>
114 
+Cc: Iker Pedrosa <ipedrosa@redhat.com>
115 
+Cc: Seth Arnold <seth.arnold@canonical.com>
116 
+Cc: Christian Brauner <christian@brauner.io>
117 
+Cc: Balint Reczey <rbalint@debian.org>
118 
+Cc: Sam James <sam@gentoo.org>
119 
+Cc: David Runge <dvzrv@archlinux.org>
120 
+Cc: Andreas Jaeger <aj@suse.de>
121 
+Cc: <~hallyn/shadow@lists.sr.ht>
122 
+Signed-off-by: Alejandro Colomar <alx@kernel.org>
123 
+---
124 
+ src/gpasswd.c | 1 +
125 
+ 1 file changed, 1 insertion(+)
126 
+
127 
+diff --git a/src/gpasswd.c b/src/gpasswd.c
128 
+index 5983f78..2d8869e 100644
129 
+--- a/src/gpasswd.c
130 
+@@ -896,6 +896,7 @@ static void change_passwd (struct group *gr)
131 
+ 		strzero (cp);
132 
+ 		cp = getpass (_("Re-enter new password: "));
133 
+ 		if (NULL == cp) {
134 
++			memzero (pass, sizeof pass);
135 
+ 			exit (1);
136 
+ 		}
137 
+
SPECS/shadow/shadow.spec
History View file @ 0f4d363
... ... 
@@ -1,7 +1,7 @@
1 1 
 Summary:        Programs for handling passwords in a secure way
2 2 
 Name:           shadow
3 3 
 Version:        4.13
4 
-Release:        4%{?dist}
4 
+Release:        5%{?dist}
5 5 
 URL:            https://github.com/shadow-maint/shadow
6 6 
 License:        BSD
7 7 
 Group:          Applications/System
... ... 
@@ -24,6 +24,10 @@ Source10: system-password
24 24 
 Source11: system-session
25 25 
 Source12: useradd
26 26
27 
+Patch0:         CVE-2023-29383.patch
28 
+Patch1:         CVE-2023-29383.1.patch
29 
+Patch2:         CVE-2023-4641.patch
30 
+
27 31 
 BuildRequires: cracklib-devel
28 32 
 BuildRequires: Linux-PAM-devel
29 33
... ... 
@@ -204,6 +208,8 @@ rm -rf %{buildroot}/*
204 204 
 %defattr(-,root,root)
205 205
206 206 
 %changelog
207 
+* Thu Dec 21 2023 Srish Srinivasan <ssrish@vmware.com> 4.13-5
208 
+- Patched CVE-2023-29383, CVE-2023-4641
207 209 
 * Sun Nov 19 2023 Shreenidhi Shedi <sshedi@vmware.com> 4.13-4
208 210 
 - Bump version as a part of openssl upgrade
209 211 
 * Wed Jan 25 2023 Shreenidhi Shedi <sshedi@vmware.com> 4.13-3