src/gpasswd.c: is_valid_user_list(): Fix invalid free(3)

This fix addresses an issue in is_valid_user_list() where the free
operation was attempted on an address not allocated with malloc().  By
duplicating the pointer with xstrdup(users) into dup, and using dup as
the original pointer, we ensure that only the valid pointer is freed,
avoiding an invalid free operation.

This bug was introduced when changing some code that used strchrnul(3)
to use strsep(3) instead.  strsep(3) advances the pointer, unlike the
previous code.

This unconditionally leads to a bug:

-  Passing NULL to free(3), if the last field in the
   colon-separated-value list is non-empty.  This results in a memory
   leak.

-  Passing a pointer to the null byte ('\0') that terminates the string,
   if the last element of the colon-separated-value list is empty.  The
   most obvious reproducer of such a bogus free(3) call is:

       free(strdup("foo:") + 4);

   This results in Undefined Behavior, and could result in allocator
   data corruption.

Fixes: 16cb664865 (2024-07-01, "lib/, src/: Use strsep(3) instead of its pattern")
Suggested-by: <https://github.com/frostb1ten>
Reported-by: <https://github.com/frostb1ten>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Brauner <christian@brauner.io>
This commit is contained in:
frostb1te
2024-11-08 05:00:24 -06:00
committed by Alejandro Colomar
parent a0771fc01a
commit 73e58adc6b

View File

@@ -174,7 +174,9 @@ static void catch_signals (int killed)
static bool is_valid_user_list (const char *users)
{
bool is_valid = true;
/*@owned@*/char *tmpusers = xstrdup (users);
char *dup, *tmpusers;
tmpusers = dup = xstrdup(users);
while (NULL != tmpusers && '\0' != *tmpusers) {
const char *u;
@@ -193,7 +195,7 @@ static bool is_valid_user_list (const char *users)
}
}
free (tmpusers);
free(dup);
return is_valid;
}