← Commit list

12a432715

Denys Vlasenko

2011-05-13

FAILED Duration: 228s

Subject

adduser: safe username passing to passwd/addgroup

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

include/libbb.h +7 -2 libbb/pw_encrypt.c +23 -2 loginutils/adduser.c +9 -7 loginutils/chpasswd.c +6 -5 loginutils/cryptpw.c +5 -22 loginutils/passwd.c +37 -38 networking/httpd.c +1 -1

include/libbb.h+7 -2

@@ -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)

libbb/pw_encrypt.c+23 -2

@@ -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*

loginutils/adduser.c+9 -7

@@ -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");
	}

loginutils/chpasswd.c+6 -5

@@ -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);
	}

loginutils/cryptpw.c+5 -22

@@ -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);

loginutils/passwd.c+37 -38

@@ -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;
	}

networking/httpd.c+1 -1

@@ -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;
	}