[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260315203903.GQ1827@brightrain.aerifal.cx>
Date: Sun, 15 Mar 2026 16:39:04 -0400
From: "dalias@...c.org" <dalias@...c.org>
To: 0rbitingZer0@...ton.me
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: Re: NSCD client getgr_a.c heap overflow + integer overflow
On Sun, Mar 15, 2026 at 06:21:52PM +0000, 0rbitingZer0@...ton.me wrote:
> Hi,
>
> During an independent source audit of musl 1.2.5 (current git master, v1.2.5-102-g1b76ff07) I found a heap overflow write in the NSCD group response parsing in src/passwd/getgr_a.c. I reported this to Rich privately, and he spotted an additional integer overflow in the same code path. He suggested moving the discussion here since no embargo is needed.
>
> Rich's assessment of the security relevance:
>
> > the only means of passing input to this interface is to be the
> > owner of /var/run/nscd/socket, a privileged path that's already
> > expected to have authority over user/group identities. However,
> > if you're running the service there with root privileges dropped
> > and perhaps some sandboxing to mitigate bugs in the nscd code
> > that make it vulnerable to attacks from remote backends, bugs in
> > musl's nscd client do enable an attacker to move from there to
> > here. So I think it's security-relevant. Not as a direct vuln
> > but as an unintended lateral movement vector.
>
> And on the fix:
>
> > The level of validation needed here was probably deemed low
> > because the source of the data is privileged. But this should
> > be fixed.
>
> Here are the details.
>
> 1. Heap overflow write (lines 127-136):
>
> The member pointer array is allocated based on GRMEMCNT (line 118):
>
> char **tmp = realloc(*mem, (groupbuf[GRMEMCNT]+1)*sizeof(char*));
>
> The loop at line 129 scans the member data for null bytes and writes a pointer for each one:
>
> for (ptr = mem[0][0], i = 0; ptr != mem[0][0]+grlist_len; ptr++)
> if (!*ptr) mem[0][++i] = ptr+1;
>
> The count check at line 133 runs after the loop completes:
>
> if (i != groupbuf[GRMEMCNT]) {
> rv = EIO;
> goto cleanup_f;
> }
>
> If the data contains more null bytes than GRMEMCNT declares, the loop writes past the allocated array before the check fires. The data shape is deliverable through the real NSCD protocol: GRMEMCNT controls how many name_len fields are read (line 80), grlist_len is their sum, and the actual data blob is fread'd at line 108. Null byte placement within the blob is fully attacker-controlled.
>
> ASan confirms: heap-buffer-overflow WRITE of size 8, 0 bytes after a 16-byte region (GRMEMCNT=1, 2-slot array, 5 null-terminated strings in data).
>
> 2. Integer overflow (lines 113-114):
>
> Rich noted:
>
> > This and lines 113 and 114 above it already have a bug:
> > integer overflow adding 1 without testing for overflow.
> > Just adding 1U isn't a solution either because then a value
> > of -1 would wrongly pass. There is just a need for more
> > validation here.
>
> Happy to post the PoC if useful.
>
> 0rbitingZer0
Does the attached look ok for a conservative fix? I think the
validation should be made more consistent in this file in general,
using a consistent interpretation of negative int32 values as invalid
vs unsigned, etc., but for now I wanted to avoid having to make
further changes like adjusting the type/signedness of the index var i,
and the attached looks like the most straightforward fix for the issue
at hand.
Also: I think assigning the count+1 to *mmem is probably wrong. It's
supposed to be the number of members not the number of slots counting
the null termination, or at least that seems to be how the callers are
using it. But if so the mistake is harmless, and I'd rather not poke
with it during "release window". Something to consider cleaning up
later though, if my analysis here is correct.
Rich
View attachment "getgr_a.diff" of type "text/plain" (1023 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
