Date: Sun, 7 Jun 2015 19:03:42 +0000 (UTC) From: Baptiste Daroussin <bapt@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r284128 - in head/usr.sbin/pw: . tests Message-ID: <201506071903.t57J3gsr040316@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bapt Date: Sun Jun 7 19:03:41 2015 New Revision: 284128 URL: https://svnweb.freebsd.org/changeset/base/284128 Log: Refactor input validation Mutualize code to validate inputs of both 'user' and 'group' command Test that the input name fits into MAXLOGNAME Modified: head/usr.sbin/pw/pw.c head/usr.sbin/pw/pw.h head/usr.sbin/pw/pw_group.c head/usr.sbin/pw/pw_user.c head/usr.sbin/pw/tests/pw_useradd.sh Modified: head/usr.sbin/pw/pw.c ============================================================================== --- head/usr.sbin/pw/pw.c Sun Jun 7 18:59:47 2015 (r284127) +++ head/usr.sbin/pw/pw.c Sun Jun 7 19:03:41 2015 (r284128) @@ -99,9 +99,11 @@ main(int argc, char *argv[]) int ch; int mode = -1; int which = -1; + long id = -1; char *config = NULL; struct stat st; - char arg; + const char *errstr; + char arg, *name; bool relocated, nis; static const char *opts[W_NUM][M_NUM] = @@ -124,12 +126,14 @@ main(int argc, char *argv[]) } }; - static int (*funcs[W_NUM]) (int _mode, struct cargs * _args) = + static int (*funcs[W_NUM]) (int _mode, char *_name, long _id, + struct cargs * _args) = { /* Request handlers */ pw_user, pw_group }; + name = NULL; relocated = nis = false; memset(&conf, 0, sizeof(conf)); strlcpy(conf.etcpath, _PATH_PWD, sizeof(conf.etcpath)); @@ -190,9 +194,15 @@ main(int argc, char *argv[]) mode = tmp % M_NUM; } else if (strcmp(argv[1], "help") == 0 && argv[2] == NULL) cmdhelp(mode, which); - else if (which != -1 && mode != -1) - addarg(&arglist, 'n', argv[1]); - else + else if (which != -1 && mode != -1) { + if (strspn(argv[1], "0123456789") == strlen(argv[1])) { + id = strtonum(argv[1], 0, LONG_MAX, &errstr); + if (errstr != NULL) + errx(EX_USAGE, "Bad id '%s': %s", + argv[1], errstr); + } else + name = argv[1]; + } else errx(EX_USAGE, "unknown keyword `%s'", argv[1]); ++argv; --argc; @@ -230,6 +240,30 @@ main(int argc, char *argv[]) case 'Y': nis = true; break; + case 'g': + if (which == 0) { /* for user* */ + addarg(&arglist, 'g', optarg); + break; + } + /* FALLTHROUGH */ + case 'u': + if (strspn(optarg, "0123456789") != strlen(optarg)) + errx(EX_USAGE, "%s expects a number", + which == 1 ? "-g" : "-u" ); + id = strtonum(optarg, 0, LONG_MAX, &errstr); + if (errstr != NULL) + errx(EX_USAGE, "Bad id '%s': %s", optarg, + errstr); + break; + case 'n': + if (strspn(optarg, "0123456789") != strlen(optarg)) { + name = optarg; + break; + } + id = strtonum(optarg, 0, LONG_MAX, &errstr); + if (errstr != NULL) + errx(EX_USAGE, "Bad id '%s': %s", optarg, + errstr); default: addarg(&arglist, ch, optarg); break; @@ -237,6 +271,9 @@ main(int argc, char *argv[]) optarg = NULL; } + if (name != NULL && strlen(name) >= MAXLOGNAME) + errx(EX_USAGE, "name too long: %s", name); + /* * Must be root to attempt an update */ @@ -265,7 +302,7 @@ main(int argc, char *argv[]) */ conf.userconf = read_userconfig(config); - ch = funcs[which] (mode, &arglist); + ch = funcs[which] (mode, name, id, &arglist); /* * If everything went ok, and we've been asked to update Modified: head/usr.sbin/pw/pw.h ============================================================================== --- head/usr.sbin/pw/pw.h Sun Jun 7 18:59:47 2015 (r284127) +++ head/usr.sbin/pw/pw.h Sun Jun 7 19:03:41 2015 (r284128) @@ -82,8 +82,8 @@ int write_userconfig(char const * file); struct carg *addarg(struct cargs * _args, int ch, char *argstr); struct carg *getarg(struct cargs * _args, int ch); -int pw_user(int mode, struct cargs * _args); -int pw_group(int mode, struct cargs * _args); +int pw_user(int mode, char *name, long id, struct cargs * _args); +int pw_group(int mode, char *name, long id, struct cargs * _args); char *pw_checkname(char *name, int gecos); int addnispwent(const char *path, struct passwd *pwd); Modified: head/usr.sbin/pw/pw_group.c ============================================================================== --- head/usr.sbin/pw/pw_group.c Sun Jun 7 18:59:47 2015 (r284127) +++ head/usr.sbin/pw/pw_group.c Sun Jun 7 19:03:41 2015 (r284128) @@ -48,12 +48,10 @@ static int print_group(struct group * gr static gid_t gr_gidpolicy(struct userconf * cnf, struct cargs * args); int -pw_group(int mode, struct cargs * args) +pw_group(int mode, char *name, long id, struct cargs * args) { int rc; struct carg *a_newname = getarg(args, 'l'); - struct carg *a_name = getarg(args, 'n'); - struct carg *a_gid = getarg(args, 'g'); struct carg *arg; struct group *grp = NULL; int grmembers = 0; @@ -68,11 +66,6 @@ pw_group(int mode, struct cargs * args) NULL }; - if (a_gid != NULL) { - if (strspn(a_gid->val, "0123456789") != strlen(a_gid->val)) - errx(EX_USAGE, "-g expects a number"); - } - if (mode == M_LOCK || mode == M_UNLOCK) errx(EX_USAGE, "'lock' command is not available for groups"); @@ -95,67 +88,63 @@ pw_group(int mode, struct cargs * args) ENDGRENT(); return EXIT_SUCCESS; } - if (a_gid == NULL) { - if (a_name == NULL) - errx(EX_DATAERR, "group name or id required"); - - if (mode != M_ADD && grp == NULL && isdigit((unsigned char)*a_name->val)) { - (a_gid = a_name)->ch = 'g'; - a_name = NULL; - } - } - grp = (a_name != NULL) ? GETGRNAM(a_name->val) : GETGRGID((gid_t) atoi(a_gid->val)); + if (id < 0 && name == NULL) + errx(EX_DATAERR, "group name or id required"); + + grp = (name != NULL) ? GETGRNAM(name) : GETGRGID(id); if (mode == M_UPDATE || mode == M_DELETE || mode == M_PRINT) { - if (a_name == NULL && grp == NULL) /* Try harder */ - grp = GETGRGID(atoi(a_gid->val)); + if (name == NULL && grp == NULL) /* Try harder */ + grp = GETGRGID(id); if (grp == NULL) { if (mode == M_PRINT && getarg(args, 'F')) { char *fmems[1]; fmems[0] = NULL; - fakegroup.gr_name = a_name ? a_name->val : "nogroup"; - fakegroup.gr_gid = a_gid ? (gid_t) atol(a_gid->val) : (gid_t)-1; + fakegroup.gr_name = name ? name : "nogroup"; + fakegroup.gr_gid = (gid_t) id; fakegroup.gr_mem = fmems; return print_group(&fakegroup); } - errx(EX_DATAERR, "unknown group `%s'", a_name ? a_name->val : a_gid->val); + if (name == NULL) + errx(EX_DATAERR, "unknown group `%s'", name); + else + errx(EX_DATAERR, "unknown group `%ld'", id); } - if (a_name == NULL) /* Needed later */ - a_name = addarg(args, 'n', grp->gr_name); + if (name == NULL) /* Needed later */ + name = grp->gr_name; /* * Handle deletions now */ if (mode == M_DELETE) { - gid_t gid = grp->gr_gid; - rc = delgrent(grp); if (rc == -1) - err(EX_IOERR, "group '%s' not available (NIS?)", grp->gr_name); + err(EX_IOERR, "group '%s' not available (NIS?)", + name); else if (rc != 0) { err(EX_IOERR, "group update"); } - pw_log(cnf, mode, W_GROUP, "%s(%u) removed", a_name->val, gid); + pw_log(cnf, mode, W_GROUP, "%s(%ld) removed", name, id); return EXIT_SUCCESS; } else if (mode == M_PRINT) return print_group(grp); - if (a_gid) - grp->gr_gid = (gid_t) atoi(a_gid->val); + if (id > 0) + grp->gr_gid = (gid_t) id; if (a_newname != NULL) grp->gr_name = pw_checkname(a_newname->val, 0); } else { - if (a_name == NULL) /* Required */ + if (name == NULL) /* Required */ errx(EX_DATAERR, "group name required"); else if (grp != NULL) /* Exists */ - errx(EX_DATAERR, "group name `%s' already exists", a_name->val); + errx(EX_DATAERR, "group name `%s' already exists", name); extendarray(&members, &grmembers, 200); members[0] = NULL; grp = &fakegroup; - grp->gr_name = pw_checkname(a_name->val, 0); + grp->gr_name = pw_checkname(name, 0); grp->gr_passwd = "*"; grp->gr_gid = gr_gidpolicy(cnf, args); grp->gr_mem = members; @@ -265,7 +254,7 @@ pw_group(int mode, struct cargs * args) grp->gr_name); else err(EX_IOERR, "group update"); - } else if (mode == M_UPDATE && (rc = chggrent(a_name->val, grp)) != 0) { + } else if (mode == M_UPDATE && (rc = chggrent(name, grp)) != 0) { if (rc == -1) errx(EX_IOERR, "group '%s' not available (NIS?)", grp->gr_name); @@ -273,9 +262,10 @@ pw_group(int mode, struct cargs * args) err(EX_IOERR, "group update"); } - arg = a_newname != NULL ? a_newname : a_name; + if (a_newname != NULL) + name = a_newname->val; /* grp may have been invalidated */ - if ((grp = GETGRNAM(arg->val)) == NULL) + if ((grp = GETGRNAM(name)) == NULL) errx(EX_SOFTWARE, "group disappeared during update"); pw_log(cnf, mode, W_GROUP, "%s(%u)", grp->gr_name, grp->gr_gid); Modified: head/usr.sbin/pw/pw_user.c ============================================================================== --- head/usr.sbin/pw/pw_user.c Sun Jun 7 18:59:47 2015 (r284127) +++ head/usr.sbin/pw/pw_user.c Sun Jun 7 19:03:41 2015 (r284128) @@ -52,7 +52,7 @@ static const char rcsid[] = static char locked_str[] = "*LOCKED*"; static int delete_user(struct userconf *cnf, struct passwd *pwd, - struct carg *a_name, int delete, int mode); + char *name, int delete, int mode); static int print_user(struct passwd * pwd); static uid_t pw_uidpolicy(struct userconf * cnf, struct cargs * args); static uid_t pw_gidpolicy(struct cargs * args, char *nam, gid_t prefer); @@ -119,13 +119,11 @@ create_and_populate_homedir(int mode, st */ int -pw_user(int mode, struct cargs * args) +pw_user(int mode, char *name, long id, struct cargs * args) { int rc, edited = 0; char *p = NULL; char *passtmp; - struct carg *a_name; - struct carg *a_uid; struct carg *arg; struct passwd *pwd = NULL; struct group *grp; @@ -166,7 +164,7 @@ pw_user(int mode, struct cargs * args) if (getarg(args, 'q')) return next; printf("%u:", next); - pw_group(mode, args); + pw_group(mode, name, -1, args); return EXIT_SUCCESS; } @@ -323,29 +321,11 @@ pw_user(int mode, struct cargs * args) return EXIT_SUCCESS; } - if ((a_name = getarg(args, 'n')) != NULL) - pwd = GETPWNAM(pw_checkname(a_name->val, 0)); - a_uid = getarg(args, 'u'); - - if (a_uid == NULL) { - if (a_name == NULL) - errx(EX_DATAERR, "user name or id required"); + if (name != NULL) + pwd = GETPWNAM(pw_checkname(name, 0)); - /* - * Determine whether 'n' switch is name or uid - we don't - * really don't really care which we have, but we need to - * know. - */ - if (mode != M_ADD && pwd == NULL - && strspn(a_name->val, "0123456789") == strlen(a_name->val) - && *a_name->val) { - (a_uid = a_name)->ch = 'u'; - a_name = NULL; - } - } else { - if (strspn(a_uid->val, "0123456789") != strlen(a_uid->val)) - errx(EX_USAGE, "-u expects a number"); - } + if (id < 0 && name == NULL) + errx(EX_DATAERR, "user name or id required"); /* * Update, delete & print require that the user exists @@ -353,22 +333,22 @@ pw_user(int mode, struct cargs * args) if (mode == M_UPDATE || mode == M_DELETE || mode == M_PRINT || mode == M_LOCK || mode == M_UNLOCK) { - if (a_name == NULL && pwd == NULL) /* Try harder */ - pwd = GETPWUID(atoi(a_uid->val)); + if (name == NULL && pwd == NULL) /* Try harder */ + pwd = GETPWUID(id); if (pwd == NULL) { if (mode == M_PRINT && getarg(args, 'F')) { - fakeuser.pw_name = a_name ? a_name->val : "nouser"; - fakeuser.pw_uid = a_uid ? (uid_t) atol(a_uid->val) : (uid_t) -1; + fakeuser.pw_name = name ? name : "nouser"; + fakeuser.pw_uid = (uid_t) id; return print_user(&fakeuser); } - if (a_name == NULL) - errx(EX_NOUSER, "no such uid `%s'", a_uid->val); - errx(EX_NOUSER, "no such user `%s'", a_name->val); + if (name == NULL) + errx(EX_NOUSER, "no such uid `%ld'", id); + errx(EX_NOUSER, "no such user `%s'", name); } - if (a_name == NULL) /* May be needed later */ - a_name = addarg(args, 'n', newstr(pwd->pw_name)); + if (name == NULL) + name = pwd->pw_name; /* * The M_LOCK and M_UNLOCK functions simply add or remove @@ -394,7 +374,7 @@ pw_user(int mode, struct cargs * args) pwd->pw_passwd += sizeof(locked_str)-1; edited = 1; } else if (mode == M_DELETE) - return (delete_user(cnf, pwd, a_name, + return (delete_user(cnf, pwd, name, getarg(args, 'r') != NULL, mode)); else if (mode == M_PRINT) return print_user(pwd); @@ -511,16 +491,16 @@ pw_user(int mode, struct cargs * args) * Add code */ - if (a_name == NULL) /* Required */ + if (name == NULL) /* Required */ errx(EX_DATAERR, "login name required"); - else if ((pwd = GETPWNAM(a_name->val)) != NULL) /* Exists */ - errx(EX_DATAERR, "login name `%s' already exists", a_name->val); + else if ((pwd = GETPWNAM(name)) != NULL) /* Exists */ + errx(EX_DATAERR, "login name `%s' already exists", name); /* * Now, set up defaults for a new user */ pwd = &fakeuser; - pwd->pw_name = a_name->val; + pwd->pw_name = name; pwd->pw_class = cnf->default_class ? cnf->default_class : ""; pwd->pw_uid = pw_uidpolicy(cnf, args); pwd->pw_gid = pw_gidpolicy(args, pwd->pw_name, (gid_t) pwd->pw_uid); @@ -635,13 +615,13 @@ pw_user(int mode, struct cargs * args) } } else if (mode == M_UPDATE || mode == M_LOCK || mode == M_UNLOCK) { if (edited) { /* Only updated this if required */ - rc = chgpwent(a_name->val, pwd); + rc = chgpwent(name, pwd); if (rc == -1) errx(EX_IOERR, "user '%s' does not exist (NIS?)", pwd->pw_name); else if (rc != 0) err(EX_IOERR, "passwd file update"); if ( cnf->nispasswd && *cnf->nispasswd=='/') { - rc = chgnispwent(cnf->nispasswd, a_name->val, pwd); + rc = chgnispwent(cnf->nispasswd, name, pwd); if (rc == -1) warn("User '%s' not found in NIS passwd", pwd->pw_name); else @@ -693,16 +673,16 @@ pw_user(int mode, struct cargs * args) /* go get a current version of pwd */ - pwd = GETPWNAM(a_name->val); + pwd = GETPWNAM(name); if (pwd == NULL) { /* This will fail when we rename, so special case that */ if (mode == M_UPDATE && (arg = getarg(args, 'l')) != NULL) { - a_name->val = arg->val; /* update new name */ - pwd = GETPWNAM(a_name->val); /* refetch renamed rec */ + name = arg->val; /* update new name */ + pwd = GETPWNAM(name); /* refetch renamed rec */ } } if (pwd == NULL) /* can't go on without this */ - errx(EX_NOUSER, "user '%s' disappeared during update", a_name->val); + errx(EX_NOUSER, "user '%s' disappeared during update", name); grp = GETGRGID(pwd->pw_gid); pw_log(cnf, mode, W_USER, "%s(%u):%s(%u):%s:%s:%s", @@ -849,7 +829,6 @@ pw_gidpolicy(struct cargs * args, char * char tmp[32]; LIST_INIT(&grpargs); - addarg(&grpargs, 'n', nam); /* * We need to auto-create a group with the user's name. We @@ -866,11 +845,11 @@ pw_gidpolicy(struct cargs * args, char * } if (conf.dryrun) { addarg(&grpargs, 'q', NULL); - gid = pw_group(M_NEXT, &grpargs); + gid = pw_group(M_NEXT, nam, -1, &grpargs); } else { - pw_group(M_ADD, &grpargs); + pw_group(M_ADD, nam, -1, &grpargs); if ((grp = GETGRNAM(nam)) != NULL) gid = grp->gr_gid; } @@ -1048,7 +1027,7 @@ pw_password(struct userconf * cnf, struc } static int -delete_user(struct userconf *cnf, struct passwd *pwd, struct carg *a_name, +delete_user(struct userconf *cnf, struct passwd *pwd, char *name, int delete, int mode) { char file[MAXPATHLEN]; @@ -1097,7 +1076,7 @@ delete_user(struct userconf *cnf, struct err(EX_IOERR, "passwd update"); if (cnf->nispasswd && *cnf->nispasswd=='/') { - rc = delnispwent(cnf->nispasswd, a_name->val); + rc = delnispwent(cnf->nispasswd, name); if (rc == -1) warnx("WARNING: user '%s' does not exist in NIS passwd", pwd->pw_name); else if (rc != 0) @@ -1105,11 +1084,11 @@ delete_user(struct userconf *cnf, struct /* non-fatal */ } - grp = GETGRNAM(a_name->val); + grp = GETGRNAM(name); if (grp != NULL && (grp->gr_mem == NULL || *grp->gr_mem == NULL) && - strcmp(a_name->val, grname) == 0) - delgrent(GETGRNAM(a_name->val)); + strcmp(name, grname) == 0) + delgrent(GETGRNAM(name)); SETGRENT(); while ((grp = GETGRENT()) != NULL) { int i, j; @@ -1118,7 +1097,7 @@ delete_user(struct userconf *cnf, struct continue; for (i = 0; grp->gr_mem[i] != NULL; i++) { - if (strcmp(grp->gr_mem[i], a_name->val) != 0) + if (strcmp(grp->gr_mem[i], name) != 0) continue; for (j = i; grp->gr_mem[j] != NULL; j++) @@ -1129,7 +1108,7 @@ delete_user(struct userconf *cnf, struct } ENDGRENT(); - pw_log(cnf, mode, W_USER, "%s(%u) account removed", a_name->val, uid); + pw_log(cnf, mode, W_USER, "%s(%u) account removed", name, uid); if (!PWALTDIR()) { /* @@ -1150,7 +1129,7 @@ delete_user(struct userconf *cnf, struct stat(home, &st) != -1) { rm_r(home, uid); pw_log(cnf, mode, W_USER, "%s(%u) home '%s' %sremoved", - a_name->val, uid, home, + name, uid, home, stat(home, &st) == -1 ? "" : "not completely "); } } Modified: head/usr.sbin/pw/tests/pw_useradd.sh ============================================================================== --- head/usr.sbin/pw/tests/pw_useradd.sh Sun Jun 7 18:59:47 2015 (r284127) +++ head/usr.sbin/pw/tests/pw_useradd.sh Sun Jun 7 19:03:41 2015 (r284128) @@ -169,12 +169,19 @@ user_add_password_expiration_date_relati atf_fail "Expiration time($TIME) was not within $EPOCH - $BUF seconds." } +atf_test_case user_add_name_too_long +user_add_name_too_long_body() { + populate_etc_skel + atf_check -e match:"too long" -s exit:64 \ + ${PW} useradd name_very_vert_very_very_very_long +} + atf_init_test_cases() { atf_add_test_case user_add atf_add_test_case user_add_noupdate atf_add_test_case user_add_comments atf_add_test_case user_add_comments_noupdate - atf_add_test_case user_add_comments_invalid + atf_add_test_case user_add_comments_invalid atf_add_test_case user_add_comments_invalid_noupdate atf_add_test_case user_add_homedir atf_add_test_case user_add_account_expiration_epoch @@ -185,4 +192,5 @@ atf_init_test_cases() { atf_add_test_case user_add_password_expiration_date_numeric atf_add_test_case user_add_password_expiration_date_month atf_add_test_case user_add_password_expiration_date_relative + atf_add_test_case user_add_name_too_long }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201506071903.t57J3gsr040316>