Bug 28752 - Segfault in getpwuid when stat fails
Summary: Segfault in getpwuid when stat fails
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: nss (show other bugs)
Version: 2.34
: P2 normal
Target Milestone: 2.36
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-06 18:30 UTC by Sam James
Modified: 2022-06-19 02:19 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
reproducer-seccomp.c (744 bytes, text/plain)
2022-01-06 18:30 UTC, Sam James
Details

Note You need to log in before you can comment on or make changes to this bug.
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!