From 546e800efcb06812c88ddf4033a1a4073203853c Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Fri, 21 May 2021 15:09:40 +0000 Subject: [PATCH] pw_scan: remove checks for (u|g)ids > USHRT_MAX. Remove the check and simplify the logic for checking UID/GID validity in `pw_scan()` by calling `strtonum` instead of `strtoul`. I ran into this because I use a non-default UID/GID numbering scheme where both are typically greater than 2^16 for normal users. The width of UIDs has been 32 bits since before 4.4BSD in 1994, almost 27 years ago; if larger UIDs were going to show up as a problem it would have already happened. Signed-off-by: Dan Cross --- lib/libc/gen/pw_scan.c | 52 ++++++++---------------------------- usr.bin/chpass/chpass.1 | 6 ----- usr.sbin/pwd_mkdb/pwd_mkdb.8 | 9 +------ usr.sbin/vipw/vipw.8 | 5 +--- 4 files changed, 13 insertions(+), 59 deletions(-) diff --git a/lib/libc/gen/pw_scan.c b/lib/libc/gen/pw_scan.c index ff97e96854..c3ec764702 100644 --- a/lib/libc/gen/pw_scan.c +++ b/lib/libc/gen/pw_scan.c @@ -49,25 +49,13 @@ #include "pw_scan.h" -/* - * Some software assumes that IDs are short. We should emit warnings - * for id's which cannot be stored in a short, but we are more liberal - * by default, warning for IDs greater than USHRT_MAX. - * - * If pw_big_ids_warning is -1 on entry to pw_scan(), it will be set based - * on the existence of PW_SCAN_BIG_IDS in the environment. - */ -static int pw_big_ids_warning = -1; - int __pw_scan(char *bp, struct passwd *pw, int flags) { - uid_t id; + long long id; int root; - char *ep, *p, *sh; - - if (pw_big_ids_warning == -1) - pw_big_ids_warning = getenv("PW_SCAN_BIG_IDS") == NULL ? 1 : 0; + char *p, *sh; + const char *ep; pw->pw_fields = 0; if (!(pw->pw_name = strsep(&bp, ":"))) /* login */ @@ -92,15 +80,10 @@ __pw_scan(char *bp, struct passwd *pw, int flags) return (0); } } - id = strtoul(p, &ep, 10); - if (errno == ERANGE) { + id = strtonum(p, 0, UID_MAX, &ep); + if (ep != NULL) { if (flags & _PWSCAN_WARN) - warnx("%s > max uid value (%lu)", p, ULONG_MAX); - return (0); - } - if (*ep != '\0') { - if (flags & _PWSCAN_WARN) - warnx("%s uid is incorrect", p); + warnx("%s uid is incorrect: %s", p, ep); return (0); } if (root && id) { @@ -108,11 +91,7 @@ __pw_scan(char *bp, struct passwd *pw, int flags) warnx("root uid should be 0"); return (0); } - if (flags & _PWSCAN_WARN && pw_big_ids_warning && id > USHRT_MAX) { - warnx("%s > recommended max uid value (%u)", p, USHRT_MAX); - /*return (0);*/ /* THIS SHOULD NOT BE FATAL! */ - } - pw->pw_uid = id; + pw->pw_uid = (uid_t)id; if (!(p = strsep(&bp, ":"))) /* gid */ goto fmt; @@ -125,22 +104,13 @@ __pw_scan(char *bp, struct passwd *pw, int flags) return (0); } } - id = strtoul(p, &ep, 10); - if (errno == ERANGE) { - if (flags & _PWSCAN_WARN) - warnx("%s > max gid value (%lu)", p, ULONG_MAX); - return (0); - } - if (*ep != '\0') { + id = strtonum(p, 0, GID_MAX, &ep); + if (ep != NULL) { if (flags & _PWSCAN_WARN) - warnx("%s gid is incorrect", p); + warnx("%s gid is incorrect: %s", p, ep); return (0); } - if (flags & _PWSCAN_WARN && pw_big_ids_warning && id > USHRT_MAX) { - warnx("%s > recommended max gid value (%u)", p, USHRT_MAX); - /* return (0); This should not be fatal! */ - } - pw->pw_gid = id; + pw->pw_gid = (gid_t)id; if (flags & _PWSCAN_MASTER ) { if (!(pw->pw_class = strsep(&bp, ":"))) /* class */ diff --git a/usr.bin/chpass/chpass.1 b/usr.bin/chpass/chpass.1 index 18b8183def..b5d6cf9bb1 100644 --- a/usr.bin/chpass/chpass.1 +++ b/usr.bin/chpass/chpass.1 @@ -255,12 +255,6 @@ When the editor terminates, the information is re-read and used to update the user database itself. Only the user, or the super-user, may edit the information associated with the user. -.Pp -See -.Xr pwd_mkdb 8 -for an explanation of the impact of setting the -.Ev PW_SCAN_BIG_IDS -environment variable. .Sh NIS INTERACTION The .Nm diff --git a/usr.sbin/pwd_mkdb/pwd_mkdb.8 b/usr.sbin/pwd_mkdb/pwd_mkdb.8 index 5dbaaef17a..5474f07207 100644 --- a/usr.sbin/pwd_mkdb/pwd_mkdb.8 +++ b/usr.sbin/pwd_mkdb/pwd_mkdb.8 @@ -121,14 +121,7 @@ The .Nm utility exits zero on success, non-zero on failure. .Sh ENVIRONMENT -If the -.Ev PW_SCAN_BIG_IDS -environment variable is set, -.Nm -will suppress the warning messages that are -normally generated for large user and group IDs. -Such IDs can cause serious problems with software -that makes assumptions about the values of IDs. +None. .Sh FILES .Bl -tag -width Pa -compact .It Pa /etc/pwd.db diff --git a/usr.sbin/vipw/vipw.8 b/usr.sbin/vipw/vipw.8 index ccb3b3e51b..9e795fcdf0 100644 --- a/usr.sbin/vipw/vipw.8 +++ b/usr.sbin/vipw/vipw.8 @@ -85,15 +85,12 @@ and the new information is not available to programs. .Sh ENVIRONMENT If the following environment variable exists it will be utilized by .Nm : -.Bl -tag -width PW_SCAN_BIG_IDS +.Bl -tag -width EDITOR .It Ev EDITOR The editor specified by the string .Ev EDITOR will be invoked instead of the default editor .Xr vi 1 . -.It Ev PW_SCAN_BIG_IDS -See -.Xr pwd_mkdb 8 . .El .Sh SEE ALSO .Xr chpass 1 , -- 2.30.1