From: frostb1te <frostb1te@bugcrowdninja.com>
Date: Fri, 8 Nov 2024 11:00:24 +0000 (-0600)
Subject: src/gpasswd.c: is_valid_user_list(): Fix invalid free(3)
X-Git-Tag: 4.17.0-rc1~35
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=73e58adc6b80c7a83d92ec9cf95ffe98db87d367;p=thirdparty%2Fshadow.git

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: 16cb66486554 (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>
---

diff --git a/src/gpasswd.c b/src/gpasswd.c
index b6b3d84f0..4fdd28122 100644
--- a/src/gpasswd.c
+++ b/src/gpasswd.c
@@ -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;
 }