adduser: safe username passing to passwd/addgroup

passwd: support creating SHA passwords
random code shrink

function                                             old     new   delta
crypt_make_pw_salt                                     -      87     +87
adduser_main                                         883     904     +21
...
crypt_make_salt                                       99      89     -10
chpasswd_main                                        329     312     -17
packed_usage                                       28731   28691     -40
passwd_main                                         1070    1000     -70
cryptpw_main                                         310     224     -86
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 4/12 up/down: 154/-288)        Total: -134 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

7 files changed, 88 insertions, 77 deletions

diff --git a/include/libbb.h b/include/libbb.h
index 4232c38f4..89e8e4452 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1259,14 +1259,19 @@ extern int correct_password(const struct passwd *pw) FAST_FUNC;
 #endif
 extern char *pw_encrypt(const char *clear, const char *salt, int cleanup) FAST_FUNC;
 extern int obscure(const char *old, const char *newval, const struct passwd *pwdp) FAST_FUNC;
-/* rnd is additional random input. New one is returned.
+/*
+ * rnd is additional random input. New one is returned.
  * Useful if you call crypt_make_salt many times in a row:
  * rnd = crypt_make_salt(buf1, 4, 0);
  * rnd = crypt_make_salt(buf2, 4, rnd);
  * rnd = crypt_make_salt(buf3, 4, rnd);
  * (otherwise we risk having same salt generated)
  */
-extern int crypt_make_salt(char *p, int cnt, int rnd) FAST_FUNC;
+extern int crypt_make_salt(char *p, int cnt /*, int rnd*/) FAST_FUNC;
+/* "$N$" + sha_salt_16_bytes + NUL */
+#define MAX_PW_SALT_LEN (3 + 16 + 1)
+extern char* crypt_make_pw_salt(char p[MAX_PW_SALT_LEN], const char *algo) FAST_FUNC;
+
 
 /* Returns number of lines changed, or -1 on error */
 #if !(ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP)
diff --git a/libbb/pw_encrypt.c b/libbb/pw_encrypt.c
index c6c04d44a..39ffa084f 100644
--- a/libbb/pw_encrypt.c
+++ b/libbb/pw_encrypt.c
@@ -27,9 +27,10 @@ static int i64c(int i)
         return ('a' - 38 + i);
 }
 
-int FAST_FUNC crypt_make_salt(char *p, int cnt, int x)
+int FAST_FUNC crypt_make_salt(char *p, int cnt /*, int x */)
 {
-        x += getpid() + time(NULL);
+        /* was: x += ... */
+        int x = getpid() + monotonic_us();
         do {
                 /* x = (x*1664525 + 1013904223) % 2^32 generator is lame
                  * (low-order bit is not "random", etc...),
@@ -47,6 +48,26 @@ int FAST_FUNC crypt_make_salt(char *p, int cnt, int x)
         return x;
 }
 
+char* FAST_FUNC crypt_make_pw_salt(char salt[MAX_PW_SALT_LEN], const char *algo)
+{
+        int len = 2/2;
+        char *salt_ptr = salt;
+        if (algo[0] != 'd') { /* not des */
+                len = 8/2; /* so far assuming md5 */
+                *salt_ptr++ = '$';
+                *salt_ptr++ = '1';
+                *salt_ptr++ = '$';
+#if !ENABLE_USE_BB_CRYPT || ENABLE_USE_BB_CRYPT_SHA
+                if (algo[0] == 's') { /* sha */
+                        salt[1] = '5' + (strcmp(algo, "sha512") == 0);
+                        len = 16/2;
+                }
+#endif
+        }
+        crypt_make_salt(salt_ptr, len);
+        return salt_ptr;
+}
+
 #if ENABLE_USE_BB_CRYPT
 
 static char*
diff --git a/loginutils/adduser.c b/loginutils/adduser.c
index 1944d9d56..a05b72158 100644
--- a/loginutils/adduser.c
+++ b/loginutils/adduser.c
@@ -82,21 +82,23 @@ static void passwd_study(struct passwd *p)
 
 static void addgroup_wrapper(struct passwd *p, const char *group_name)
 {
-        char *argv[5];
+        char *argv[6];
 
         argv[0] = (char*)"addgroup";
         if (group_name) {
                 /* Add user to existing group */
-                argv[1] = p->pw_name;
-                argv[2] = (char*)group_name;
-                argv[3] = NULL;
+                argv[1] = (char*)"--";
+                argv[2] = p->pw_name;
+                argv[3] = (char*)group_name;
+                argv[4] = NULL;
         } else {
                 /* Add user to his own group with the first free gid found in passwd_study */
 //TODO: to be compatible with external addgroup programs we should use --gid instead...
                 argv[1] = (char*)"-g";
                 argv[2] = utoa(p->pw_gid);
-                argv[3] = p->pw_name;
-                argv[4] = NULL;
+                argv[3] = (char*)"--";
+                argv[4] = p->pw_name;
+                argv[5] = NULL;
         }
 
         spawn_and_wait(argv);
@@ -106,7 +108,7 @@ static void passwd_wrapper(const char *login_name) NORETURN;
 
 static void passwd_wrapper(const char *login_name)
 {
-        BB_EXECLP("passwd", "passwd", login_name, NULL);
+        BB_EXECLP("passwd", "passwd", "--", login_name, NULL);
         bb_error_msg_and_die("can't execute passwd, you must set password manually");
 }
 
diff --git a/loginutils/chpasswd.c b/loginutils/chpasswd.c
index 6c4296faa..f4718c829 100644
--- a/loginutils/chpasswd.c
+++ b/loginutils/chpasswd.c
@@ -37,9 +37,8 @@ int chpasswd_main(int argc UNUSED_PARAM, char **argv)
         char *name, *pass;
         char salt[sizeof("$N$XXXXXXXX")];
         int opt, rc;
-        int rnd = rnd; /* we *want* it to be non-initialized! */
 
-        if (getuid())
+        if (getuid() != 0)
                 bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
 
         opt_complementary = "m--e:e--m";
@@ -55,10 +54,12 @@ int chpasswd_main(int argc UNUSED_PARAM, char **argv)
                 xuname2uid(name); /* dies if there is no such user */
 
                 if (!(opt & OPT_ENC)) {
-                        rnd = crypt_make_salt(salt, 1, rnd);
+                        crypt_make_salt(salt, 1);
                         if (opt & OPT_MD5) {
-                                strcpy(salt, "$1$");
-                                rnd = crypt_make_salt(salt + 3, 4, rnd);
+                                salt[0] = '$';
+                                salt[1] = '1';
+                                salt[2] = '$';
+                                crypt_make_salt(salt + 3, 4);
                         }
                         pass = pw_encrypt(pass, salt, 0);
                 }
diff --git a/loginutils/cryptpw.c b/loginutils/cryptpw.c
index bbaa858da..b25a39ac9 100644
--- a/loginutils/cryptpw.c
+++ b/loginutils/cryptpw.c
@@ -19,7 +19,7 @@
 //usage:        IF_LONG_OPTS(
 //usage:     "\n        -P,--password-fd=N      Read password from fd N"
 /* //usage:  "\n        -s,--stdin              Use stdin; like -P0" */
-//usage:     "\n        -m,--method=TYPE        Encryption method TYPE"
+//usage:     "\n        -m,--method=TYPE        Encryption method"
 //usage:     "\n        -S,--salt=SALT"
 //usage:        )
 //usage:        IF_NOT_LONG_OPTS(
@@ -39,7 +39,7 @@
 //usage:        IF_LONG_OPTS(
 //usage:     "\n        -P,--password-fd=N      Read password from fd N"
 /* //usage:  "\n        -s,--stdin              Use stdin; like -P0" */
-//usage:     "\n        -m,--method=TYPE        Encryption method TYPE"
+//usage:     "\n        -m,--method=TYPE        Encryption method"
 //usage:     "\n        -S,--salt=SALT"
 //usage:        )
 //usage:        IF_NOT_LONG_OPTS(
@@ -92,11 +92,9 @@ to cryptpw. -a option (alias for -m) came from cryptpw.
 int cryptpw_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int cryptpw_main(int argc UNUSED_PARAM, char **argv)
 {
-        /* $N$ + sha_salt_16_bytes + NUL */
-        char salt[3 + 16 + 1];
+        char salt[MAX_PW_SALT_LEN];
         char *salt_ptr;
         const char *opt_m, *opt_S;
-        int len;
         int fd;
 
 #if ENABLE_LONG_OPTS
@@ -121,24 +119,9 @@ int cryptpw_main(int argc UNUSED_PARAM, char **argv)
         if (argv[0] && !opt_S)
                 opt_S = argv[1];
 
-        len = 2/2;
-        salt_ptr = salt;
-        if (opt_m[0] != 'd') { /* not des */
-                len = 8/2; /* so far assuming md5 */
-                *salt_ptr++ = '$';
-                *salt_ptr++ = '1';
-                *salt_ptr++ = '$';
-#if !ENABLE_USE_BB_CRYPT || ENABLE_USE_BB_CRYPT_SHA
-                if (opt_m[0] == 's') { /* sha */
-                        salt[1] = '5' + (strcmp(opt_m, "sha512") == 0);
-                        len = 16/2;
-                }
-#endif
-        }
+        salt_ptr = crypt_make_pw_salt(salt, opt_m);
         if (opt_S)
-                safe_strncpy(salt_ptr, opt_S, sizeof(salt) - 3);
-        else
-                crypt_make_salt(salt_ptr, len, 0);
+                safe_strncpy(salt_ptr, opt_S, sizeof(salt) - (sizeof("$N$")-1));
 
         xmove_fd(fd, STDIN_FILENO);
 
diff --git a/loginutils/passwd.c b/loginutils/passwd.c
index 810644e61..8c47e65ff 100644
--- a/loginutils/passwd.c
+++ b/loginutils/passwd.c
@@ -9,7 +9,7 @@
 //usage:       "Change USER's password. If no USER is specified,\n"
 //usage:       "changes the password for the current user.\n"
 //usage:     "\nOptions:"
-//usage:     "\n        -a ALG  Algorithm to use for password (des, md5)" /* ", sha1)" */
+//usage:     "\n        -a ALG  Encryption method"
 //usage:     "\n        -d      Delete password for the account"
 //usage:     "\n        -l      Lock (disable) account"
 //usage:     "\n        -u      Unlock (re-enable) account"
@@ -22,15 +22,15 @@ static void nuke_str(char *str)
         if (str) memset(str, 0, strlen(str));
 }
 
-static char* new_password(const struct passwd *pw, uid_t myuid, int algo)
+static char* new_password(const struct passwd *pw, uid_t myuid, const char *algo)
 {
-        char salt[sizeof("$N$XXXXXXXX")]; /* "$N$XXXXXXXX" or "XX" */
+        char salt[MAX_PW_SALT_LEN];
         char *orig = (char*)"";
         char *newp = NULL;
         char *cp = NULL;
         char *ret = NULL; /* failure so far */
 
-        if (myuid && pw->pw_passwd[0]) {
+        if (myuid != 0 && pw->pw_passwd[0]) {
                 char *encrypted;
 
                 orig = bb_ask_stdin("Old password: "); /* returns ptr to static */
@@ -38,13 +38,13 @@ static char* new_password(const struct passwd *pw, uid_t myuid, int algo)
                         goto err_ret;
                 encrypted = pw_encrypt(orig, pw->pw_passwd, 1); /* returns malloced str */
                 if (strcmp(encrypted, pw->pw_passwd) != 0) {
-                        syslog(LOG_WARNING, "incorrect password for %s",
-                                pw->pw_name);
+                        syslog(LOG_WARNING, "incorrect password for %s", pw->pw_name);
                         bb_do_delay(LOGIN_FAIL_DELAY);
                         puts("Incorrect password");
                         goto err_ret;
                 }
-                if (ENABLE_FEATURE_CLEAN_UP) free(encrypted);
+                if (ENABLE_FEATURE_CLEAN_UP)
+                        free(encrypted);
         }
         orig = xstrdup(orig); /* or else bb_ask_stdin() will destroy it */
         newp = bb_ask_stdin("New password: "); /* returns ptr to static */
@@ -52,22 +52,22 @@ static char* new_password(const struct passwd *pw, uid_t myuid, int algo)
                 goto err_ret;
         newp = xstrdup(newp); /* we are going to bb_ask_stdin() again, so save it */
         if (ENABLE_FEATURE_PASSWD_WEAK_CHECK
-         && obscure(orig, newp, pw) && myuid)
+         && obscure(orig, newp, pw)
+         && myuid != 0
+        ) {
                 goto err_ret; /* non-root is not allowed to have weak passwd */
+        }
 
         cp = bb_ask_stdin("Retype password: ");
         if (!cp)
                 goto err_ret;
-        if (strcmp(cp, newp)) {
+        if (strcmp(cp, newp) != 0) {
                 puts("Passwords don't match");
                 goto err_ret;
         }
 
-        crypt_make_salt(salt, 1, 0); /* des */
-        if (algo) { /* MD5 */
-                strcpy(salt, "$1$");
-                crypt_make_salt(salt + 3, 4, 0);
-        }
+        crypt_make_pw_salt(salt, algo);
+
         /* pw_encrypt returns malloced str */
         ret = pw_encrypt(newp, salt, 1);
         /* whee, success! */
@@ -75,8 +75,10 @@ static char* new_password(const struct passwd *pw, uid_t myuid, int algo)
  err_ret:
         nuke_str(orig);
         if (ENABLE_FEATURE_CLEAN_UP) free(orig);
+
         nuke_str(newp);
         if (ENABLE_FEATURE_CLEAN_UP) free(newp);
+
         nuke_str(cp);
         return ret;
 }
@@ -85,17 +87,15 @@ int passwd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int passwd_main(int argc UNUSED_PARAM, char **argv)
 {
         enum {
-                OPT_algo = 0x1, /* -a - password algorithm */
-                OPT_lock = 0x2, /* -l - lock account */
-                OPT_unlock = 0x4, /* -u - unlock account */
-                OPT_delete = 0x8, /* -d - delete password */
-                OPT_lud = 0xe,
-                STATE_ALGO_md5 = 0x10,
-                //STATE_ALGO_des = 0x20, not needed yet
+                OPT_algo   = (1 << 0), /* -a - password algorithm */
+                OPT_lock   = (1 << 1), /* -l - lock account */
+                OPT_unlock = (1 << 2), /* -u - unlock account */
+                OPT_delete = (1 << 3), /* -d - delete password */
+                OPT_lud    = OPT_lock | OPT_unlock | OPT_delete,
         };
         unsigned opt;
         int rc;
-        const char *opt_a = "";
+        const char *opt_a = "d"; /* des */
         const char *filename;
         char *myname;
         char *name;
@@ -116,13 +116,9 @@ int passwd_main(int argc UNUSED_PARAM, char **argv)
         //argc -= optind;
         argv += optind;
 
-        if (strcasecmp(opt_a, "des") != 0) /* -a */
-                opt |= STATE_ALGO_md5;
-        //else
-        //      opt |= STATE_ALGO_des;
         myuid = getuid();
         /* -l, -u, -d require root priv and username argument */
-        if ((opt & OPT_lud) && (myuid || !argv[0]))
+        if ((opt & OPT_lud) && (myuid != 0 || !argv[0]))
                 bb_show_usage();
 
         /* Will complain and die if username not found */
@@ -130,7 +126,7 @@ int passwd_main(int argc UNUSED_PARAM, char **argv)
         name = argv[0] ? argv[0] : myname;
 
         pw = xgetpwnam(name);
-        if (myuid && pw->pw_uid != myuid) {
+        if (myuid != 0 && pw->pw_uid != myuid) {
                 /* LOGMODE_BOTH */
                 bb_error_msg_and_die("%s can't change password for %s", myname, name);
         }
@@ -164,27 +160,29 @@ int passwd_main(int argc UNUSED_PARAM, char **argv)
         newp = NULL;
         c = pw->pw_passwd[0] - '!';
         if (!(opt & OPT_lud)) {
-                if (myuid && !c) { /* passwd starts with '!' */
+                if (myuid != 0 && !c) { /* passwd starts with '!' */
                         /* LOGMODE_BOTH */
                         bb_error_msg_and_die("can't change "
                                         "locked password for %s", name);
                 }
                 printf("Changing password for %s\n", name);
-                newp = new_password(pw, myuid, opt & STATE_ALGO_md5);
+                newp = new_password(pw, myuid, opt_a);
                 if (!newp) {
                         logmode = LOGMODE_STDIO;
                         bb_error_msg_and_die("password for %s is unchanged", name);
                 }
         } else if (opt & OPT_lock) {
-                if (!c) goto skip; /* passwd starts with '!' */
+                if (!c)
+                        goto skip; /* passwd starts with '!' */
                 newp = xasprintf("!%s", pw->pw_passwd);
         } else if (opt & OPT_unlock) {
-                if (c) goto skip; /* not '!' */
+                if (c)
+                        goto skip; /* not '!' */
                 /* pw->pw_passwd points to static storage,
                  * strdup'ing to avoid nasty surprizes */
                 newp = xstrdup(&pw->pw_passwd[1]);
         } else if (opt & OPT_delete) {
-                newp = (char*)""; //xstrdup("");
+                newp = (char*)"";
         }
 
         rlimit_fsize.rlim_cur = rlimit_fsize.rlim_max = 512L * 30000;
@@ -202,7 +200,7 @@ int passwd_main(int argc UNUSED_PARAM, char **argv)
         rc = update_passwd(bb_path_shadow_file, name, newp, NULL);
         if (rc > 0)
                 /* password in /etc/shadow was updated */
-                newp = (char*) "x"; //xstrdup("x");
+                newp = (char*) "x";
         if (rc >= 0)
                 /* 0 = /etc/shadow missing (not an error), >0 = passwd changed in /etc/shadow */
 #endif
@@ -212,16 +210,17 @@ int passwd_main(int argc UNUSED_PARAM, char **argv)
         }
         /* LOGMODE_BOTH */
         if (rc < 0)
-                bb_error_msg_and_die("can't update password file %s",
-                                filename);
+                bb_error_msg_and_die("can't update password file %s", filename);
         bb_info_msg("Password for %s changed by %s", name, myname);
 
-        //if (ENABLE_FEATURE_CLEAN_UP) free(newp);
+        /*if (ENABLE_FEATURE_CLEAN_UP) free(newp); - can't, it may be non-malloced */
  skip:
         if (!newp) {
                 bb_error_msg_and_die("password for %s is already %slocked",
                         name, (opt & OPT_unlock) ? "un" : "");
         }
-        if (ENABLE_FEATURE_CLEAN_UP) free(myname);
+
+        if (ENABLE_FEATURE_CLEAN_UP)
+                free(myname);
         return 0;
 }
diff --git a/networking/httpd.c b/networking/httpd.c
index d6157aca2..d77342a2a 100644
--- a/networking/httpd.c
+++ b/networking/httpd.c
@@ -2424,7 +2424,7 @@ int httpd_main(int argc UNUSED_PARAM, char **argv)
                 salt[0] = '$';
                 salt[1] = '1';
                 salt[2] = '$';
-                crypt_make_salt(salt + 3, 4, 0);
+                crypt_make_salt(salt + 3, 4);
                 puts(pw_encrypt(pass, salt, 1));
                 return 0;
         }