Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zw6_0fzKlkBIbRSj@itl-email>
Date: Tue, 15 Oct 2024 15:17:34 -0400
From: Demi Marie Obenour <demi@...isiblethingslab.com>
To: oss-security@...ts.openwall.com
Subject: Re: CVE-2024-47191: Local root exploit in the PAM
 module pam_oath.so

On Tue, Oct 15, 2024 at 01:43:42PM +0200, Matthias Gerstner wrote:
> Hi,
> 
> thanks for bringing up the potential problems with the patch we (SUSE)
> suggested. The missing drop of the ancillary group list has indeed been
> overlooked and will result in a lack of protection, since the
> "unprivileged" process will likely still be a member of the root group.
> 
> I will adjust the patch to contain this and one or two other adjustments
> and can then share it again here on the list.
> 
> Please find a few more comments below inline.
> 
> On Tue, Oct 08, 2024 at 10:56:59PM +0200, Solar Designer wrote:
> > On Tue, Oct 08, 2024 at 08:10:17AM +0200, Simon Josefsson wrote:
> > > I noticed that that there are Linux-PAM helpers to drop privileges:
> > > 
> > > https://github.com/linux-pam/linux-pam/blob/master/libpam/pam_modutil_priv.c#L52
> > 
> > This currently switches fsuid/fsgid (so for the current thread only),
> > but uses initgroups() and setgroups() libc functions (so affects all
> > threads).
> 
> Regarding thread safety, the SUSE patch forks a new process to drop the
> privileges, so it shouldn't be an issue here.
> 
> > In particular, I worry that the SUSE approach could be susceptible to
> > hard link attacks (when the fs.protected_hardlinks sysctl is not set).
> > Would this allow to overwrite someone else's file (the original issue)
> > if the user can hard link that file?  I currently don't see why not, so
> > it's probably a vulnerability.
> 
> Indeed the patch does not take care of hard link attacks. Our products
> don't have any supported configuration without protected_hardlinks
> enabled, so we didn't have this in mind.
> 
> The change to address this concern should be rather small, though, so I
> will try to incorporate it in a new version of the patch.
> 
> > In general, switching to a user not only drops privileges for file
> > access, but also potentially exposes the process as that user's.
> > fsuid/fsgid switching is the safest in this respect (these were meant
> > just for file access purposes), but with other IDs (depending on which)
> > there may be extra exposure of the partially privileged log in process
> > to the user via /proc, kill(), setpriority(), etc. ... but thankfully
> > and hopefully not also via ptrace() on modern systems anymore.
> 
> On modern Linux there shouldn't be a problem with dropping UID/GID, as
> the kernel will set the process's suid_dumpable attribute to the setting
> found in sys.fs.suid_dumpable, which should be 0. When this happens no
> ptrace() etc. will be possible on the end on the user/group that the
> process drops privileges to.
> 
> It can be problematic when the unprivileged process subsequently
> performs an execve() without closing sensitive file descriptors, like it
> happened in open-vm-tools (CVE-2023-34059).
> 
> An explicit prctl(PR_SETDUMPABLE, 0) could be considered in the patch to
> make this requirement explicit.
> 
> Dropping only the fsuid and fsgid on Linux would avoid any potential
> ptrace() dangers. The system calls are marked deprecated, though,
> and have unfortunate error handling. What makes me feel a bit uneasy
> about this approach is that the programmer has to make sure that
> the privilege drop context only ever deals with file system operations.
> Things like e.g. obtaining SO_PEERCREDs from a UNIX domain socket will
> still operate on the egid and euid, which will remain at 0.
> 
> For me it feels better to drop all privileges for good. For specific
> purposes like in the case of pam-oath I believe it can make sense to
> take that route, though.
> 
> Best Regards
> 
> Matthias

What about opening the path one portion at a time using openat() with
O_NOFOLLOW (and, as applicable, O_DIRECTORY), ensuring that each portion
is not "." or "..", does not contain "/", and is owned by either the
target user or root?  This solves all race conditions and does not
require spawning another process.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.