VesperOS/shadow
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lib/chkname.c: is_valid_user_name(): Remove unnecessary check

Browse Source 
If (maxsize == -1), then ((size_t)maxsize == SIZE_MAX).  And no size can
ever be >= SIZE_MAX, so it will never return false if sysconf(3) reports
an unlimited user-name size via returning -1.  Well, to be pedantic,
that disallows a user-name siz of precisely SIZE_MAX bytes when
sysconf(3) returns -1.  However, that's probably a good thing; such a
long user name might trigger Undefined Behavior somewhere else, so be
cautious and disallow it.  I hope nobody will be using the entire
address space for a user name.

The commit that introduced that check missed that this code had always
supported unlimited user-name sizes since it was introduced by Iker in
3b7cc05387 ("lib: replace `USER_NAME_MAX_LENGTH` macro"), and
6be85b0baf ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare
warning") even clarified this in the commit message.

So, while the code in 6a1f45d932 ("lib/chkname.c: Support unlimited
user name lengths") wasn't bad per se, the commit message was incorrect.
What that patch did was adding code for handling EINVAL (or any other
errors that a future kernel might add).

To be more pedantically correct, that commit also allowed (under certain
circumstances, user names of SIZE_MAX bytes, but those were originally
allowed (by accident), and only became disallowed in 403a2e3771
("lib/chkname.c: Take NUL byte into account").  But again, let's
disallow those, just to be cautious.

Link: <https://github.com/shadow-maint/shadow/pull/935>
Link: <https://github.com/shadow-maint/shadow/pull/935#discussion_r1477429492>
See-also: 6be85b0baf ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare warning")
Fixes: 6a1f45d932 ("lib/chkname.c: Support unlimited user name lengths")
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Tobias Stoeckmann <tobias@stoeckmann.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This commit is contained in:
Alejandro Colomar
2024-02-13 16:13:05 -06:00
committed by Serge Hallyn
parent 15882a5f90
commit ad307ee42a
1 changed files with 12 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/chkname.c
+12 -11
View File
@@ -1,11 +1,9 @@
/*
* SPDX-FileCopyrightText: 1990 - 1994, Julianne Frances Haugh
* SPDX-FileCopyrightText: 1996 - 2000, Marek Michałkiewicz
* SPDX-FileCopyrightText: 2001 - 2005, Tomasz Kłoczko
* SPDX-FileCopyrightText: 2005 - 2008, Nicolas François
*
* SPDX-License-Identifier: BSD-3-Clause
*/
// SPDX-FileCopyrightText: 1990-1994, Julianne Frances Haugh
// SPDX-FileCopyrightText: 1996-2000, Marek Michałkiewicz
// SPDX-FileCopyrightText: 2001-2005, Tomasz Kłoczko
// SPDX-FileCopyrightText: 2005-2008, Nicolas François
// SPDX-FileCopyrightText: 2023-2024, Alejandro Colomar <alx@kernel.org>
// SPDX-License-Identifier: BSD-3-Clause
/*
* is_valid_user_name(), is_valid_group_name() - check the new user/group
@@ -74,7 +72,9 @@ static bool is_valid_name (const char *name)
return !numeric;
}
bool is_valid_user_name (const char *name)
bool
is_valid_user_name(const char *name)
{
long maxsize;
@@ -82,12 +82,14 @@ bool is_valid_user_name (const char *name)
maxsize = sysconf(_SC_LOGIN_NAME_MAX);
if (maxsize == -1 && errno != 0)
maxsize = LOGIN_NAME_MAX;
if (maxsize != -1 && strlen(name) >= (size_t)maxsize)
if (strlen(name) >= (size_t)maxsize)
return false;
return is_valid_name (name);
}
bool is_valid_group_name (const char *name)
{
/*
@@ -101,4 +103,3 @@ bool is_valid_group_name (const char *name)
return is_valid_name (name);
}