Bug 1212214

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?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Once you get the leak, a program should read all the free memory
searching for patterns that gpasswd(1) leaves nearby the leaked
password.  That's probably much easier than triggering the password leak
itself.

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 (maybe by malloc(3)ing
the entire system memory), right after the victim leaks their password.

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 (while I believe it's not so hard
to find the password once it's been leaked, it's not easy to provoke the
leak).  Nevertheless, 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: Christian Brauner <christian@brauner.io>
Cc: Seth Arnold <seth.arnold@canonical.com>
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>
---

Hi Serge,

Since we seem to agree that it's not easily exploitable, I'm sending the
patch publicly to the list, while also CCing many distro maintainers so
they can pick it up soon if they wish.

I documented all we know about it in the commit message, so everyone has
enough information to evaluate it by themselves.

The diff below also contains more than usual, to see the bug easily in
the patch.

Have a lovely weekend!
Alex

 src/gpasswd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gpasswd.c b/src/gpasswd.c
index 609fe0a4..3b76ff8e 100644
--- a/src/gpasswd.c
+++ b/src/gpasswd.c
@@ -886,30 +886,31 @@ static void change_passwd (struct group *gr)
 	 * identical. There is no need to validate the old password since
 	 * the invoker is either the group owner, or root.
 	 */
 	printf (_("Changing the password for group %s\n"), group);
 
 	for (retries = 0; retries < RETRIES; retries++) {
 		cp = agetpass (_("New Password: "));
 		if (NULL == cp) {
 			exit (1);
 		}
 
 		STRFCPY (pass, cp);
 		erase_pass (cp);
 		cp = agetpass (_("Re-enter new password: "));
 		if (NULL == cp) {
+			memzero (pass, sizeof pass);
 			exit (1);
 		}
 
 		if (strcmp (pass, cp) == 0) {
 			erase_pass (cp);
 			break;
 		}
 
 		erase_pass (cp);
 		memzero (pass, sizeof pass);
 
 		if (retries + 1 < RETRIES) {
 			puts (_("They don't match; try again"));
 		}
 	}

Ressources:
https://github.com/shadow-maint/shadow/commit/65c88a43a23c2391dcc90c0abda3e839e9c57904