[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZxDKuqteocmdBDNx@kasco.suse.de>
Date: Thu, 17 Oct 2024 10:28:41 +0200
From: Matthias Gerstner <mgerstner@...e.de>
To: oss-security@...ts.openwall.com
Subject: Re: CVE-2024-47191: Local root exploit in the PAM
module pam_oath.so
Hi,
On Tue, Oct 15, 2024 at 10:21:35PM +0200, Solar Designer wrote:
> On Tue, Oct 15, 2024 at 03:17:34PM -0400, Demi Marie Obenour wrote:
> > What about opening the path one portion at a time using openat() with
> > O_NOFOLLOW (and, as applicable, O_DIRECTORY),
>
> As I understand, the SUSE patch already does that.
Exactly. The privilege-drop is only an extra hardening. The patch
first attempts to securely determine the ownership of the
oath-usersfile, then drops privileges to the owner of the file for
further processing.
> Both the SUSE and the upstream patch try to do two things at once: drop
> privileges and access files safer. Maybe I missed, but I think neither
> of you specified whether these two things are intended as required
> complementary parts (and what attacks would work if only one part worked
> as intended) or as defense-in-depth. You could want to clarify this.
The privilege-drop in our path is defense-in-depth. The file operations
used should be robust on their own already. Dealing with file system
APIs securely is such contexts is quite a complex task as can be seen in
the discussion of the for this bugfix, so adding the extra layer of
dropping privileges makes me feel better about this.
> > I will adjust the patch to contain this and one or two other adjustments
> > and can then share it again here on the list.
An updated version of the patch is attached to this email. As is stated
at the end of the patch description, the following changes have been
made compared to the previously shared version:
- setgroups() is invoked to drop supplementary group membership. Without
this the forked process wrongly retains the root group membership.
Since the privilege drop is only an additional hardening measure, the
original patch should still prove safe.
- the usersfile is checked for additional hard-links; if the link count
is larger than one, then the file is rejected. This prevents possible
hard link attacks on the end of the unprivileged user. With the Linux
kernel sysctl protected_hardlinks set to 1 (the usual default on most
distributions), this attack will not work either way.
- O_NOCTTY has been added to the open() call of the usersfile. This
makes this aspect explicit, although the code already checks that the
file is a regular file, so the situation shouldn't arise in the first
place.
> > 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.
>
> I guess it's checks of st_uid and st_nlink? There are still some really
> subtle issues like side effects on open() of a device file, which can be
> hard linked too, but privilege dropping should mostly prevent them. I'd
> also add O_NOCTTY.
In our case it is only a check of st_nlink. This is because we are
dropping privileges to the owner of the file. If one would drop
privileges to the to-be-authenticated user, then a check of st_uid would
be in order as well.
The reason why we accept different ownership of the file is for
increased backward compatibility. The usersfile feature in pam-oath
allows for potentially complex scenarios regarding to the ownership of
the file and it was not previously clearly specified which scenarios are
supported. When only supporting the simple scenario of the usersfile
being located directly beneath the to-be-authenticated user's home
directory, then a lot of things become simpler, as it has been done
in the upstream approach in a couple of aspects.
Best Regards
Matthias
View attachment "0001-usersfile-fix-potential-security-issues-in-PAM-modul.patch" of type "text/plain" (30135 bytes)
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.
