[Pkg-shadow-devel] [Git][debian/adduser][master] 10 commits: add check_user_comment
Marc Haber (@zugschlus)
gitlab at salsa.debian.org
Tue May 6 19:15:29 BST 2025
Marc Haber pushed to branch master at Debian / adduser
Commits:
f0986465 by Marc Haber at 2025-05-05T19:47:51+02:00
add check_user_comment
Git-Dch: ignore
- - - - -
4f14d633 by Marc Haber at 2025-05-05T21:11:03+02:00
implement user_comment test, comment tests that currently fail
Git-Dch: ignore
- - - - -
57392211 by Marc Haber at 2025-05-05T21:11:03+02:00
stop sanitizing new_comment
Git-Dch: ignore
- - - - -
9dcdd8b3 by Marc Haber at 2025-05-05T21:11:03+02:00
mark comment variable as tainted in the name
Git-Dch: ignore
- - - - -
0eb5a59b by Marc Haber at 2025-05-05T21:11:03+02:00
re-work ch_comment function
Git-Dch: ignore
the function no longer accesses the global $new_name
the function no longer uses chfn but sets the complete field
- - - - -
66b1d5a3 by Marc Haber at 2025-05-06T10:14:41+02:00
unconditionally sanitize comment right before calling usermod
Git-Dch: ignore
- - - - -
aff96082 by Marc Haber at 2025-05-06T10:14:41+02:00
adapt autopkgtests to not expect ,,, in comment field
Git-Dch: ignore
- - - - -
6842a9b5 by Marc Haber at 2025-05-06T10:14:41+02:00
activate all those strange characters in comment test
Git-Dch: ignore
- - - - -
48ca8ba8 by Marc Haber at 2025-05-06T19:24:56+02:00
add failing test to comment test10.pl
Git-Dch: ignore
- - - - -
dd3c9a8e by Marc Haber at 2025-05-06T19:25:22+02:00
check comment for control chars and :
Git-Dch: ignore
- - - - -
4 changed files:
- adduser
- debian/tests/lib/AdduserTestsCommon.pm
- testsuite/lib_test.pm
- testsuite/test10.pl
Changes:
=====================================
adduser
=====================================
@@ -117,7 +117,7 @@ our $found_sys_opt = undef;
our $ingroup_name = undef;
our $new_firstgid = undef;
our $new_firstuid = undef;
-our $new_comment = undef;
+our $comment_tainted = undef;
our $gid_option = undef;
our $primary_gid = undef;
our $new_lastgid = undef;
@@ -160,7 +160,7 @@ GetOptions(
'allow-all-names' => sub { $name_check_level = 2 },
'allow-badname' => sub { $name_check_level = 1 unless $name_check_level },
'allow-bad-names' => sub { $name_check_level = 1 unless $name_check_level },
- 'comment:s' => \$new_comment,
+ 'comment:s' => \$comment_tainted,
'conf|c=s' => \@configfiles,
'debug' => sub { $verbose = 2; },
'stdoutmsglevel=s' => \$stdoutmsglevel,
@@ -171,7 +171,7 @@ GetOptions(
'firstgid=i' => \$new_firstgid,
'firstuid=i' => \$new_firstuid,
'force-badname' => sub { $name_check_level = 1 unless $name_check_level },
- 'gecos:s' => \$new_comment,
+ 'gecos:s' => \$comment_tainted,
'gid=i' => \$gid_option,
'group' => \$found_group_opt,
'help|h' => sub { &usage; exit 0; },
@@ -357,16 +357,18 @@ if( defined $special_home ) {
$special_home = sanitize_string( decode($charset, $special_home), simplepathre);
}
+if ( defined $comment_tainted ) {
+ log_trace("check comment %s for unwanted chars", $special_home);
+ # do not sanitize, can't be done without libperl
+ if ( $comment_tainted !~ qr/^([^\x00-\x1F\x7F:]*)$/ ) {
+ die( "unwanted chars in comment" );
+ }
+}
if( defined $special_shell ) {
log_trace("sanitize special_shell");
$special_shell = sanitize_string( decode($charset, $special_shell), simplepathre);
}
-if( defined $new_comment ) {
- log_trace("sanitize new_comment");
- $new_comment = sanitize_string( decode($charset, $new_comment), commentre);
-}
-
if( defined $new_firstgid ) {
log_trace("sanitize new_firstgid");
$new_firstgid = sanitize_string($new_firstgid, numberre);
@@ -412,7 +414,7 @@ $SIG{'INT'} = $SIG{'QUIT'} = $SIG{'HUP'} = 'handler';
# $action = "adduser"
# $new_name - the name of the new user.
# $ingroup_name | $gid_option - the group to add the user to
-# $special_home, $new_uid, $new_comment - optional overrides
+# $special_home, $new_uid, $comment_tainted - optional overrides
# $action = "addgroup"
# $new_name - the name of the new group
# $gid_option - optional override
@@ -422,7 +424,7 @@ $SIG{'INT'} = $SIG{'QUIT'} = $SIG{'HUP'} = 'handler';
# $action = "addsysuser"
# $new_name - the name of the new user
# $make_group_also | $ingroup_name | $gid_option | 0 - which group
-# $special_home, $new_uid, $new_comment - optional overrides
+# $special_home, $new_uid, $comment_tainted - optional overrides
# $action = "addusertogroup"
# $existing_user - the user to be added
# $existing_group - the group to add her to
@@ -678,10 +680,10 @@ if ($action eq "addsysuser") {
release_lock(0);
- if (defined($new_comment)) {
- ch_comment($new_comment);
+ if (defined($comment_tainted)) {
+ ch_comment($new_name, $comment_tainted);
} elsif ($uid_pool{$new_name}{'comment'}) {
- ch_comment($uid_pool{$new_name}{'comment'});
+ ch_comment($new_name, $uid_pool{$new_name}{'comment'});
}
$primary_gid = $gid_option;
@@ -972,10 +974,10 @@ if ($action eq "adduser") {
}
}
- if (defined($new_comment)) {
- ch_comment($new_comment);
+ if (defined($comment_tainted)) {
+ ch_comment($new_name, $comment_tainted);
} elsif ($uid_pool{$new_name}{'comment'}) {
- ch_comment($uid_pool{$new_name}{'comment'});
+ ch_comment($new_name, $uid_pool{$new_name}{'comment'});
} else {
my $noexpr = langinfo(NOEXPR());
my $yesexpr = langinfo(YESEXPR());
@@ -1480,21 +1482,15 @@ sub first_avail_uid_gid {
}
sub ch_comment {
- my $chfn = &which('chfn');
- my $comment = shift;
- if($comment =~ /,/) {
- my($comment_name,$comment_room,$comment_work,$comment_home,$comment_other)
- = split(/,/,$comment);
-
- systemcall($chfn, '-f', $comment_name, '-r', $comment_room, $new_name);
- systemcall($chfn,'-w',$comment_work,$new_name)
- if(defined($comment_work));
- systemcall($chfn,'-h',$comment_home,$new_name)
- if(defined($comment_home));
- systemcall($chfn,'-o',$comment_other,$new_name)
- if(defined($comment_other));
+ my ($name, $comment) = @_;
+ my $usermod = &which('usermod');
+
+ # untaint unconditionally. our call to system() is safe, so
+ # we leave the check to usermod
+ if ($comment =~ qr/^([^\x00-\x1F\x7F:]*)$/ ) {
+ systemcall($usermod, '-c', $1, $name);
} else {
- systemcall($chfn, '-f', $comment, $new_name);
+ log_fatal("unconditional sanitize of comment failed. This should not happen.");
}
}
=====================================
debian/tests/lib/AdduserTestsCommon.pm
=====================================
@@ -387,7 +387,6 @@ sub assert_user_has_home_directory {
sub assert_user_has_comment {
my ($user, $comment) = @_;
- $comment .= ',,,';
is((egetpwnam($user))[6], $comment, "user has comment: ~$user is $comment");
}
=====================================
testsuite/lib_test.pm
=====================================
@@ -163,6 +163,13 @@ sub check_user_homedir_eq {
return ($userdir eq $dir) ? 0 : 1;
}
+sub check_user_comment {
+ my ($username, $comment) = @_;
+ my $usercomment = (getpwnam($username))[6];
+
+ return ($usercomment eq $comment) ? 0 : 1;
+}
+
sub check_user_homedir_not_exist {
my ($username) = @_;
my $dir = (getpwnam($username))[7];
=====================================
testsuite/test10.pl
=====================================
@@ -5,39 +5,52 @@ use strict;
use lib_test;
my $username = find_unused_name();
-# ths strange characters here are UTF-8, this will cause problems
-# on systems that don't have libperl5.40 installed.
-#my $cmd = 'adduser --comment="é ä O\'Leary œŒ àœæßéÀÔùñ" --disabled-password '. "$username";
-my $cmd = 'adduser --comment="Tom O\'Malley" --disabled-password '. "$username";
-
-my %config;
-
-my @adduserconf=("/etc/adduser.conf");
-preseed_config(\@adduserconf,\%config);
-
-if (!defined (getpwnam($username))) {
- print "Testing $cmd... ";
- `$cmd`;
- my $error = ($?>>8);
- if ($error) {
- print "failed\n adduser returned an errorcode != 0 ($error)\n";
- exit $error;
- }
- assert(check_user_exist ($username));
-
+my $comment;
+my $cmd;
+
+sub testusercomment {
+ my ($username, $comment, $fail_expected) = @_;
+ $fail_expected ||= 0;
+ $cmd = 'adduser --comment="'. $comment. '" --home=/nonexistent --disabled-password '. "$username";
+ if (!defined (getpwnam($username))) {
+ print "Testing $cmd... ";
+ `$cmd`;
+ my $error = ($?>>8);
+ if( $fail_expected > 0 ) {
+ assert(check_user_not_exist ($username));
+ } else {
+ if ($error) {
+ print "failed\n adduser returned an errorcode != 0 ($error)\n";
+ exit $error;
+ }
+ assert(check_user_exist ($username));
+ assert(check_user_comment ($username, $comment));
+ }
+
+ }
+
+ $cmd = "deluser $username";
+ if (defined (getpwnam($username))) {
+ print "Testing $cmd... ";
+ `$cmd`;
+ my $error = ($?>>8);
+ if ($error) {
+ print "failed\n adduser returned an errorcode != 0 ($error)\n";
+ exit $error;
+ }
+ assert(check_user_not_exist ($username));
+ print "ok\n";
+ }
}
-$cmd = "deluser $username";
-if (defined (getpwnam($username))) {
- print "Testing $cmd... ";
- `$cmd`;
- my $error = ($?>>8);
- if ($error) {
- print "failed\n adduser returned an errorcode != 0 ($error)\n";
- exit $error;
- }
- assert(check_user_not_exist ($username));
- print "ok\n";
- `rm -rf /home/$username`;
-}
+testusercomment($username, "Tom");
+testusercomment($username, "Tom Omalley");
+testusercomment($username, "Tom O\'Malley");
+testusercomment($username, "Tom O\'Mälléy");
+testusercomment($username, "Tomaß O\'Mälléy");
+testusercomment($username, "Éom O\'Mälléy");
+testusercomment($username, "Éoœm O\'Mälléy");
+testusercomment($username, "Tom:Malley", 1);
+
+# vim: tabstop=4 shiftwidth=4 expandtab
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/d310aef944dd7faaf0a8b19b2ab5781a5043a32b...dd3c9a8e015902f54f310e1d37e8df56ad86b928
--
View it on GitLab: https://salsa.debian.org/debian/adduser/-/compare/d310aef944dd7faaf0a8b19b2ab5781a5043a32b...dd3c9a8e015902f54f310e1d37e8df56ad86b928
You're receiving this email because of your account on salsa.debian.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-shadow-devel/attachments/20250506/0179965d/attachment-0001.htm>
More information about the Pkg-shadow-devel
mailing list