Bug 28752

Summary: Segfault in getpwuid when stat fails
Product: glibc Reporter: Sam James <sam>
Component: nssAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: dj, fweimer
Priority: P2 Flags: fweimer: security-
Version: 2.34   
Target Milestone: 2.36   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=2084588
Host: Target:
Build: Last reconfirmed:
Attachments: reproducer-seccomp.c

Description Sam James 2022-01-06 18:30:52 UTC
Created attachment 13894 [details]
reproducer-seccomp.c

Originally reported in Gentoo: https://bugs.gentoo.org/828070
Discussed on libc-help here: https://sourceware.org/pipermail/libc-help/2021-December/006061.html

glib compiled with FAM support ends up crashing Firefox. Andreas Fink did some substantial debugging and ended up finding that the issue is that stat (in nss_database_check_reload_and_get) may fail when e.g. newfstatat is forbidden by a seccomp filter.

I've attached Andreas' reproducer here. Needs to be linked against libseccomp.

azanella had a simple patch which works for me:
```
diff --git a/nss/nss_database.c b/nss/nss_database.c
index d56c5b798d..24e34213cd 100644
--- a/nss/nss_database.c
+++ b/nss/nss_database.c
@@ -424,10 +424,11 @@ nss_database_check_reload_and_get (struct nss_database_state *local,
      errors here are very unlikely, but the chance that we're entering
      a container is also very unlikely, so we err on the side of both
      very unlikely things not happening at the same time.  */
-  if (__stat64_time64 ("/", &str) != 0
-      || (local->root_ino != 0
-         && (str.st_ino != local->root_ino
-             ||  str.st_dev != local->root_dev)))
+  if (__stat64_time64 ("/", &str) != 0)
+    return false;
+
+  if (local->root_ino != 0 && (str.st_ino != local->root_ino
+                              || str.st_dev != local->root_dev))
     {
       /* Change detected; disable reloading and return current state.  */
       atomic_store_release (&local->data.reload_disabled, 1);
```
Comment 1 Florian Weimer 2022-01-07 09:56:56 UTC
Thanks. The comment needs updating as well.
Comment 2 Sam James 2022-03-14 17:04:49 UTC
Updated patch sent to libc-alpha: https://patchwork.sourceware.org/project/glibc/patch/20220314165414.3110670-2-sam@gentoo.org/.
Comment 3 Sam James 2022-06-09 05:34:34 UTC
Fixed in master with 3fdf0a205b622e40fa7e3c4ed1e4ed4d5c6c5380 and ace9e3edbca62d978b1e8f392d8a5d78500272d9.

I'm not au fait with the workflow yet for glibc's Bugzilla, so I'll call this WAITING based on the fact we haven't backported it yet, and it's a good candidate for doing so after some time to soak.
Comment 4 Sam James 2022-06-09 05:35:22 UTC
(In reply to Sam James from comment #3)
> Fixed in master with 3fdf0a205b622e40fa7e3c4ed1e4ed4d5c6c5380 and
> ace9e3edbca62d978b1e8f392d8a5d78500272d9.
>

Sorry, thought it'd linkify:

commit ace9e3edbca62d978b1e8f392d8a5d78500272d9 (origin/master, origin/HEAD, master)
Author: Sam James <sam@gentoo.org>
Date:   Sun Jun 5 04:57:10 2022 +0100

    nss: handle stat failure in check_reload_and_get (BZ #28752)

    Skip the chroot test if the database isn't loaded
    correctly (because the chroot test uses some
    existing DB state).

    The __stat64_time64 -> fstatat call can fail if
    running under an (aggressive) seccomp filter,
    like Firefox seems to use.

    This manifested in a crash when using glib built
    with FAM support with such a Firefox build.

    Suggested-by: DJ Delorie <dj@redhat.com>
    Signed-off-by: Sam James <sam@gentoo.org>
    Reviewed-by: DJ Delorie <dj@redhat.com>

commit 3fdf0a205b622e40fa7e3c4ed1e4ed4d5c6c5380
Author: Sam James <sam@gentoo.org>
Date:   Sun Jun 5 04:57:09 2022 +0100

    nss: add assert to DB_LOOKUP_FCT (BZ #28752)

    It's interesting if we have a null action list,
    so an assert is worthwhile.

    Suggested-by: DJ Delorie <dj@redhat.com>
    Signed-off-by: Sam James <sam@gentoo.org>
    Reviewed-by: DJ Delorie <dj@redhat.com>
Comment 5 Sam James 2022-06-19 02:18:52 UTC
Backports are done to 2.34 & 2.35. Earlier versions were unaffected.

Thanks all!