Attachment 185282 Details for Bug 221416 – pw useradd: Validate the user name before creating the entry

[patch] pw useradd: Validate the user name before creating the entry

pw-useradd-validate-user-name.diff (text/plain), 5.67 KB, created by Fabian Keil on 2017-08-11 11:42:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Fabian Keil

Created: 2017-08-11 11:42:44 UTC

Size: 5.67 KB

patch

obsolete

>From 0f9cc4e11065a590307162faed564325ae438b15 Mon Sep 17 00:00:00 2001
>From: Fabian Keil <fk@fabiankeil.de>
>Date: Tue, 8 Aug 2017 12:02:37 +0200
>Subject: [PATCH 1/3] pw useradd: Validate the user name before creating the
> entry
>
>Previouly it was possible to create users with spaces in the name with:
>pw useradd -u 1234 -g 1234 -n 'test user'
>
>The "-g 1234" is relevant, without it the name was already rejected
>as expected:
>
>[fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
>pw: invalid character ` ' at position 4 in userid/group name
>
>Bug unintentionally found with a salt config without explicit name entry:
>
>test user:
>  user.present:
>    - uid: 1234
>    - gid: 1234
>    - fullname: Test user
>    - shell: /usr/local/bin/bash
>    - home: /home/test
>    - groups:
>      - wheel
>      - salt
>
>"Luckily" salt modules rarely bother with input validation either ...
>
>Obtained from: ElectroBSD
>---
> usr.sbin/pw/pw_user.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/usr.sbin/pw/pw_user.c b/usr.sbin/pw/pw_user.c
>index 73f43ea43865..a5c334fb2d72 100644
>--- a/usr.sbin/pw/pw_user.c
>+++ b/usr.sbin/pw/pw_user.c
>@@ -1202,7 +1202,7 @@ pw_user_add(int argc, char **argv, char *arg1)
> 		if (arg1[strspn(arg1, "0123456789")] == '\0')
> 			id = pw_checkid(arg1, UID_MAX);
> 		else
>-			name = arg1;
>+			name = pw_checkname(arg1, 0);
> 	}
> 
> 	while ((ch = getopt(argc, argv, args)) != -1) {
>@@ -1214,7 +1214,7 @@ pw_user_add(int argc, char **argv, char *arg1)
> 			quiet = true;
> 			break;
> 		case 'n':
>-			name = optarg;
>+			name = pw_checkname(optarg, 0);
> 			break;
> 		case 'u':
> 			userid = optarg;
>-- 
>2.13.2
>
>
>From 59effa6b075d26f6f8f3144b6748357427b62916 Mon Sep 17 00:00:00 2001
>From: Fabian Keil <fk@fabiankeil.de>
>Date: Tue, 8 Aug 2017 14:30:24 +0200
>Subject: [PATCH 2/3] usr.sbin/pw/tests: Add test to confirm that user names
> with spaces are rejected
>
>... if no group id is specified.
>
>Obtained from: ElectroBSD
>---
> usr.sbin/pw/tests/pw_useradd_test.sh | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/usr.sbin/pw/tests/pw_useradd_test.sh b/usr.sbin/pw/tests/pw_useradd_test.sh
>index d14e4dde82ee..4303f2ad4cb0 100755
>--- a/usr.sbin/pw/tests/pw_useradd_test.sh
>+++ b/usr.sbin/pw/tests/pw_useradd_test.sh
>@@ -176,6 +176,18 @@ user_add_name_too_long_body() {
> 		${PW} useradd name_very_vert_very_very_very_long
> }
> 
>+atf_test_case user_add_name_with_spaces
>+user_add_name_with_spaces_body() {
>+	populate_etc_skel
>+	atf_check -s exit:65 -e match:"invalid character" \
>+		  ${PW} useradd 'test user'
>+	atf_check -s exit:1 -o empty grep "^test user:.*" $HOME/master.passwd
>+	# Try again with the -n option which uses a slightly different code path.
>+	atf_check -s exit:65 -e match:"invalid character" \
>+		  ${PW} useradd -n 'test user'
>+	atf_check -s exit:1 -o empty grep "^test user:.*" $HOME/master.passwd
>+}
>+
> atf_test_case user_add_expiration
> user_add_expiration_body() {
> 	populate_etc_skel
>@@ -415,6 +427,7 @@ atf_init_test_cases() {
> 	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
>+	atf_add_test_case user_add_name_with_spaces
> 	atf_add_test_case user_add_expiration
> 	atf_add_test_case user_add_invalid_user_entry
> 	atf_add_test_case user_add_invalid_group_entry
>-- 
>2.13.2
>
>
>From dd2654d86452761ca3a570d20a86e91bd710744b Mon Sep 17 00:00:00 2001
>From: Fabian Keil <fk@fabiankeil.de>
>Date: Tue, 8 Aug 2017 14:44:12 +0200
>Subject: [PATCH 3/3] usr.sbin/pw/tests: Add test to verify that invalid user
> names are caught
>
>... even if the -g option is specified.
>
>Obtained from: ElectroBSD
>---
> usr.sbin/pw/tests/pw_useradd_test.sh | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
>diff --git a/usr.sbin/pw/tests/pw_useradd_test.sh b/usr.sbin/pw/tests/pw_useradd_test.sh
>index 4303f2ad4cb0..1569b06585c0 100755
>--- a/usr.sbin/pw/tests/pw_useradd_test.sh
>+++ b/usr.sbin/pw/tests/pw_useradd_test.sh
>@@ -188,6 +188,31 @@ user_add_name_with_spaces_body() {
> 	atf_check -s exit:1 -o empty grep "^test user:.*" $HOME/master.passwd
> }
> 
>+atf_test_case user_add_name_with_spaces_and_gid_specified
>+user_add_name_with_spaces_and_gid_specified_body() {
>+	populate_etc_skel
>+	gid=12345
>+	user_name="test user"
>+	# pw useradd should fail because of the space in the user
>+	# name, not because the group doesn't exist.
>+	atf_check -s exit:65 -e match:"invalid character" \
>+		  ${PW} useradd "${user_name}" -g ${gid}
>+	atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
>+	# Try again with the -n option which uses a slightly different code path.
>+	atf_check -s exit:65 -e match:"invalid character" \
>+		  ${PW} useradd -n "${user_name}" -g ${gid}
>+	atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
>+	# Make sure the user isn't added even if the group exists
>+	atf_check -s exit:0 ${PW} groupadd blafasel -g ${gid}
>+	atf_check -s exit:65 -e match:"invalid character" \
>+		  ${PW} useradd "${user_name}" -g ${gid}
>+	atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
>+	# Try again with the -n option.
>+	atf_check -s exit:65 -e match:"invalid character" \
>+		  ${PW} useradd -n "${user_name}" -g ${gid}
>+	atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
>+}
>+
> atf_test_case user_add_expiration
> user_add_expiration_body() {
> 	populate_etc_skel
>@@ -428,6 +453,7 @@ atf_init_test_cases() {
> 	atf_add_test_case user_add_password_expiration_date_relative
> 	atf_add_test_case user_add_name_too_long
> 	atf_add_test_case user_add_name_with_spaces
>+	atf_add_test_case user_add_name_with_spaces_and_gid_specified
> 	atf_add_test_case user_add_expiration
> 	atf_add_test_case user_add_invalid_user_entry
> 	atf_add_test_case user_add_invalid_group_entry
>-- 
>2.13.2
>

Actions: View | Diff

Attachments on bug 221416: 185282