From af8a9dfc9a193bc5ddc96c6d338dc4332a50cc40 Mon Sep 17 00:00:00 2001 From: bapt Date: Sat, 4 Jul 2015 15:27:04 +0000 Subject: [PATCH] Validate input of pw usermod -h and pwusermod -H Push the code that set the password into a separate function to improve readability Add regression tests about pw usermod -h and pw usermod -H --- usr.sbin/pw/pw.c | 30 ++++++++ usr.sbin/pw/pw_user.c | 123 ++++++++++++++++---------------- usr.sbin/pw/pwupd.h | 2 + usr.sbin/pw/tests/pw_usermod.sh | 37 ++++++++++ 4 files changed, 132 insertions(+), 60 deletions(-) diff --git a/usr.sbin/pw/pw.c b/usr.sbin/pw/pw.c index 30fb55b4f61d..b9bd9d052cfe 100644 --- a/usr.sbin/pw/pw.c +++ b/usr.sbin/pw/pw.c @@ -137,6 +137,7 @@ main(int argc, char *argv[]) relocated = nis = false; memset(&conf, 0, sizeof(conf)); strlcpy(conf.etcpath, _PATH_PWD, sizeof(conf.etcpath)); + conf.fd = -1; LIST_INIT(&arglist); @@ -280,6 +281,35 @@ main(int argc, char *argv[]) errx(EX_USAGE, "Bad id '%s': %s", optarg, errstr); break; + case 'H': + if (conf.fd != -1) + errx(EX_USAGE, "'-h' and '-H' are mutually " + "exclusive options"); + conf.precrypted = true; + if (strspn(optarg, "0123456789") != strlen(optarg)) + errx(EX_USAGE, "'-H' expects a file descriptor"); + + conf.fd = strtonum(optarg, 0, INT_MAX, &errstr); + if (errstr != NULL) + errx(EX_USAGE, "Bad file descriptor '%s': %s", + optarg, errstr); + break; + case 'h': + if (conf.fd != -1) + errx(EX_USAGE, "'-h' and '-H' are mutually " + "exclusive options"); + + if (strcmp(optarg, "-") == 0) + conf.fd = '-'; + else if (strspn(optarg, "0123456789") == strlen(optarg)) { + conf.fd = strtonum(optarg, 0, INT_MAX, &errstr); + if (errstr != NULL) + errx(EX_USAGE, "'-h' expects a " + "file descriptor or '-'"); + } else + errx(EX_USAGE, "'-h' expects a file " + "descriptor or '-'"); + break; case 'o': conf.checkduplicate = true; break; diff --git a/usr.sbin/pw/pw_user.c b/usr.sbin/pw/pw_user.c index c3b2751e26a8..f1dbadcd1d43 100644 --- a/usr.sbin/pw/pw_user.c +++ b/usr.sbin/pw/pw_user.c @@ -86,6 +86,67 @@ create_and_populate_homedir(int mode, struct passwd *pwd) pwd->pw_uid, pwd->pw_dir); } +static int +set_passwd(struct passwd *pwd, struct carg *arg, bool update) +{ + int b, istty; + struct termios t, n; + login_cap_t *lc; + char line[_PASSWORD_LEN+1]; + char *p; + + if (conf.fd == '-') { + if (!pwd->pw_passwd || *pwd->pw_passwd != '*') { + pwd->pw_passwd = "*"; /* No access */ + return (1); + } + return (0); + } + + if ((istty = isatty(conf.fd))) { + if (tcgetattr(conf.fd, &t) == -1) + istty = 0; + else { + n.c_lflag &= ~(ECHO); + tcsetattr(conf.fd, TCSANOW, &n); + printf("%s%spassword for user %s:", + update ? "new " : "", + conf.precrypted ? "encrypted " : "", + pwd->pw_name); + fflush(stdout); + } + } + b = read(conf.fd, line, sizeof(line) - 1); + if (istty) { /* Restore state */ + tcsetattr(conf.fd, TCSANOW, &t); + fputc('\n', stdout); + fflush(stdout); + } + + if (b < 0) + err(EX_IOERR, "-%c file descriptor", + conf.precrypted ? 'H' : 'h'); + line[b] = '\0'; + if ((p = strpbrk(line, "\r\n")) != NULL) + *p = '\0'; + if (!*line) + errx(EX_DATAERR, "empty password read on file descriptor %d", + conf.fd); + if (conf.precrypted) { + if (strchr(line, ':') != NULL) + return EX_DATAERR; + pwd->pw_passwd = line; + } else { + lc = login_getpwclass(pwd); + if (lc == NULL || + login_setcryptfmt(lc, "sha512", NULL) == NULL) + warn("setting crypt(3) format"); + login_close(lc); + pwd->pw_passwd = pw_pwcrypt(line); + } + return (1); +} + /*- * -C config configuration file * -q quiet operation @@ -529,66 +590,8 @@ pw_user(int mode, char *name, long id, struct cargs * args) } } - if ((arg = getarg(args, 'h')) != NULL || - (arg = getarg(args, 'H')) != NULL) { - if (strcmp(arg->val, "-") == 0) { - if (!pwd->pw_passwd || *pwd->pw_passwd != '*') { - pwd->pw_passwd = "*"; /* No access */ - edited = 1; - } - } else { - int fd = atoi(arg->val); - int precrypt = (arg->ch == 'H'); - int b; - int istty = isatty(fd); - struct termios t; - login_cap_t *lc; - - if (istty) { - if (tcgetattr(fd, &t) == -1) - istty = 0; - else { - struct termios n = t; - - /* Disable echo */ - n.c_lflag &= ~(ECHO); - tcsetattr(fd, TCSANOW, &n); - printf("%s%spassword for user %s:", - (mode == M_UPDATE) ? "new " : "", - precrypt ? "encrypted " : "", - pwd->pw_name); - fflush(stdout); - } - } - b = read(fd, line, sizeof(line) - 1); - if (istty) { /* Restore state */ - tcsetattr(fd, TCSANOW, &t); - fputc('\n', stdout); - fflush(stdout); - } - if (b < 0) - err(EX_IOERR, "-%c file descriptor", - precrypt ? 'H' : 'h'); - line[b] = '\0'; - if ((p = strpbrk(line, "\r\n")) != NULL) - *p = '\0'; - if (!*line) - errx(EX_DATAERR, "empty password read on file descriptor %d", fd); - if (precrypt) { - if (strchr(line, ':') != NULL) - return EX_DATAERR; - pwd->pw_passwd = line; - } else { - lc = login_getpwclass(pwd); - if (lc == NULL || - login_setcryptfmt(lc, "sha512", NULL) == NULL) - warn("setting crypt(3) format"); - login_close(lc); - pwd->pw_passwd = pw_pwcrypt(line); - } - edited = 1; - } - } + if (conf.fd != -1) + edited = set_passwd(pwd, arg, mode == M_UPDATE); /* * Special case: -N only displays & exits diff --git a/usr.sbin/pw/pwupd.h b/usr.sbin/pw/pwupd.h index 8f46e7d5ca52..c6ed32e6c40f 100644 --- a/usr.sbin/pw/pwupd.h +++ b/usr.sbin/pw/pwupd.h @@ -85,10 +85,12 @@ struct pwconf { char etcpath[MAXPATHLEN]; char *newname; char *config; + int fd; bool dryrun; bool pretty; bool v7; bool checkduplicate; + bool precrypted; struct userconf *userconf; }; diff --git a/usr.sbin/pw/tests/pw_usermod.sh b/usr.sbin/pw/tests/pw_usermod.sh index dbc6481a202e..006bb2c17328 100755 --- a/usr.sbin/pw/tests/pw_usermod.sh +++ b/usr.sbin/pw/tests/pw_usermod.sh @@ -119,6 +119,41 @@ user_mod_rename_too_long_body() { -l name_very_very_very_very_very_long } +atf_test_case user_mod_h +user_mod_h_body() { + populate_etc_skel + + atf_check -s exit:0 ${PW} useradd foo + atf_check -s exit:0 ${PW} usermod foo -h 0 <<- EOF + $(echo a) + EOF + atf_check -s exit:0 -o not-match:"^foo:\*:.*" \ + grep "^foo" ${HOME}/master.passwd + atf_check -s exit:0 ${PW} usermod foo -h - <<- EOF + $(echo b) + EOF + atf_check -s exit:0 -o match:"^foo:\*:.*" \ + grep "^foo" ${HOME}/master.passwd + atf_check -e inline:"pw: '-h' expects a file descriptor or '-'\n" \ + -s exit:64 ${PW} usermod foo -h a <<- EOF + $(echo a) + EOF +} + +atf_test_case user_mod_H +user_mod_H_body() { + populate_etc_skel + + atf_check -s exit:0 ${PW} useradd foo + atf_check -s exit:0 ${PW} usermod foo -H 0 <<- EOF + $(echo a) + EOF + atf_check -s exit:0 -o match:"^foo:a:.*" \ + grep "^foo" ${HOME}/master.passwd + atf_check -s exit:64 -e inline:"pw: '-H' expects a file descriptor\n" \ + ${PW} usermod foo -H - +} + atf_init_test_cases() { atf_add_test_case user_mod atf_add_test_case user_mod_noupdate @@ -130,4 +165,6 @@ atf_init_test_cases() { atf_add_test_case user_mod_name_noupdate atf_add_test_case user_mod_rename atf_add_test_case user_mod_rename_too_long + atf_add_test_case user_mod_h + atf_add_test_case user_mod_H }