]> git.ipfire.org Git - thirdparty/shadow.git/commitdiff
lib/chkname.c: is_valid_user_name(): Remove unnecessary check
authorAlejandro Colomar <alx@kernel.org>
Mon, 5 Feb 2024 13:14:01 +0000 (14:14 +0100)
committerSerge Hallyn <serge@hallyn.com>
Tue, 13 Feb 2024 22:13:05 +0000 (16:13 -0600)
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
3b7cc053872c ("lib: replace `USER_NAME_MAX_LENGTH` macro"), and
6be85b0bafb5 ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare
warning") even clarified this in the commit message.

So, while the code in 6a1f45d932c8 ("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 403a2e3771be
("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: 6be85b0bafb5 ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare warning")
Fixes: 6a1f45d932c8 ("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>
lib/chkname.c

index 433eb5e69a1055bf4ac4c295573c4e4d1f734b4c..79fa29c36ab7df1e908ca98e1a86b20a68c6999b 100644 (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);
 }
-