From: Alejandro Colomar Date: Mon, 5 Feb 2024 13:14:01 +0000 (+0100) Subject: lib/chkname.c: is_valid_user_name(): Remove unnecessary check X-Git-Tag: 4.15.0-rc2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ad307ee42aff04245b154c71506309c0244e412a;p=thirdparty%2Fshadow.git lib/chkname.c: is_valid_user_name(): Remove unnecessary check 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: Link: 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 Cc: Tobias Stoeckmann Cc: Serge Hallyn Signed-off-by: Alejandro Colomar --- diff --git a/lib/chkname.c b/lib/chkname.c index 433eb5e69..79fa29c36 100644 --- a/lib/chkname.c +++ b/lib/chkname.c @@ -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 +// 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); } -