From: Serge Hallyn <serge@hallyn.com>
Date: Tue, 21 Jul 2026 19:41:09 +0000 (-0500)
Subject: useradd: don't mask user_id variable in check_uid_range
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=HEAD;p=thirdparty%2Fshadow.git

useradd: don't mask user_id variable in check_uid_range

While it's not "wrong" right now, it could be confusing, and cause
trouble later.

I'm not a fan of global variables in the first place, but restructuring
venerable code like this tends to be asking for trouble.

Also rename rflg as sys_user.  And fix its type, as it is a bool, not
int.

Also a note on commit 764e56e9ec: useradd: fix subuid allocation,
which is in a PR comment.  Putting it here so it'll be in the git
tree itself:

> For posterity: this was apparently always broken, but the opposite way. The
> user_id part of the check was always true if !user_id, and user_id was always
> 0, so that was ignored. Alex fixed that accidentally by switching to if
> (user_id == 0) return false, in 8508d61.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
Reported-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
---

diff --git a/src/useradd.c b/src/useradd.c
index a56d73712..fd8e36b8a 100644
--- a/src/useradd.c
+++ b/src/useradd.c
@@ -254,7 +254,7 @@ static void usr_update (unsigned long subuid_count, unsigned long subgid_count,
                         const struct option_flags *flags);
 static void create_home(const struct option_flags *flags);
 static void create_mail(const struct option_flags *flags);
-static void check_uid_range(int rflg, uid_t user_id);
+static void check_uid_range(bool sys_user, uid_t uid);
 
 
 /*
@@ -2339,21 +2339,21 @@ static void create_mail(const struct option_flags *flags)
 #endif
 }
 
-static void check_uid_range(int rflg, uid_t user_id)
+static void check_uid_range(bool sys_user, uid_t uid)
 {
 	uid_t uid_min ;
 	uid_t uid_max ;
-	if (rflg) {
+	if (sys_user) {
 		uid_max = getdef_ulong("SYS_UID_MAX",getdef_ulong("UID_MIN",1000UL)-1);
-		if (user_id > uid_max) {
-			eprintf(_("%s warning: %s's uid %d is greater than SYS_UID_MAX %d\n"), Prog, user_name, user_id, uid_max);
+		if (uid > uid_max) {
+			eprintf(_("%s warning: %s's uid %d is greater than SYS_UID_MAX %d\n"), Prog, user_name, uid, uid_max);
 		}
 	}else{
 		uid_min = getdef_ulong("UID_MIN", 1000UL);
 		uid_max = getdef_ulong("UID_MAX", 6000UL);
 		if (uid_min <= uid_max) {
-			if (user_id < uid_min || user_id >uid_max)
-				eprintf(_("%s warning: %s's uid %d outside of the UID_MIN %d and UID_MAX %d range.\n"), Prog, user_name, user_id, uid_min, uid_max);
+			if (uid < uid_min || uid > uid_max)
+				eprintf(_("%s warning: %s's uid %d outside of the UID_MIN %d and UID_MAX %d range.\n"), Prog, user_name, uid, uid_min, uid_max);
 		}
 	}
 
@@ -2543,7 +2543,7 @@ int main (int argc, char **argv)
 #endif				/* ENABLE_SUBIDS */
 
 	if (uflg)
-	   check_uid_range(rflg,user_id);
+	   check_uid_range(rflg, user_id);
 #ifdef WITH_TCB
 	if (getdef_bool ("USE_TCB")) {
 		if (shadowtcb_create (user_name, user_id) == SHADOWTCB_FAILURE) {