[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [oss-security] Possible memory leak on getspnam / getspnam_r

Hi Jean,

On Tue, Aug 24, 2021 at 08:14:02PM -0300, Jean Diogo wrote:
> The function getspnam() and it's reentrant sister getspnam_r() do not clean
> the content of allocated memory before returning to the user, resulting in
> the leak of /etc/shadow content. In some cases this might be an issue.

There's no reliable way for a program to ensure nothing sensitive is
left in memory.  However, the library can make a better effort to make
it unlikely that password hashes would be left in memory.  Like you
suggested in another message (somehow detached from this thread),
endspent() could be a place to zeroize any knowingly cached sensitive
data, although it would only be usable that way by programs, not by
higher-level libraries where thread-safety matters.

> Let me put ProFTPd as an example: it's daemon starts as root, when it
> receives a connection it forks, opens all the files that it'll need, then
> it calls getspnam with the provided FTP user to validate the provided
> password. Later on it setreuid(nobody) abandoning root privileges [1]. Here
> it doesn't matter whether it calls getspnam or getspnam_r, the malloced
> buffer will remain on heap memory (and it's pointer in libc <buffer> and
> <respbuf>, I suppose). So now the child process is running on a low
> privileged user and has a copy of /etc/shadow on it's heap memory.
> The vulnerability CVE-2020-9273 [2] (an use-after-free on heap) allowed me
> to get RCE on ProFTPd, in an exploit I created last year. Additionally,
> thanks to getspnam caching it is possible to read the root cryptogram (and
> other users).
> I'm not suggesting that ProFTPd architecture is correct [3]. The problem is
> that even if ProFTPd calls getspnam_r it has no mechanism to zero the cache
> before forking, since internal pointers are not known to the user (read
> developer).

> [3] - It is a good programming practice to exec right after fork.

An alternative programming practice is to fork() a child process for
authentication, then let that child process (with sensitive data in it)
terminate and have the service proceed to authenticated state (with
nothing from /etc/shadow having ever been loaded into its memory).  This
is what I implemented in popa3d from the start, see its auth_shadow.c

			 startup as root
			|child          |parent
			v               v
	drop to user popa3d,            still as root,
	handle the AUTHORIZATION        wait for and
	state, write the results, - - > read the authentication
	and exit                        information
			|child          |parent
			v               v
	getspnam(3), crypt(3),          wait for and
	check, write the result,  - - > read the authentication
	and exit (to clean up)          result
					drop to the authenticated user,
					handle the TRANSACTION state,
					possibly UPDATE the mailbox,
					and exit

This is also what we have in pam_tcb, enabled with the "fork" option:

       fork   Create child processes for accessing shadow files.   Using  this
              option  one can be sure that after a call to pam_end(3) there is
              no sensitive data left in the process' address space.   However,
              this  option  may  confuse some of the more complicated applica-
              tions and it has some performance overhead.

Maybe we should finally get pam_tcb into Linux-PAM, now that it no
longer depends on custom glibc patches since libxcrypt finally provides
our crypt_gensalt*() API.  Maybe we can have it fully replace pam_unix
in there, just like it had on Owl and ALT Linux 20 years ago.