From 045c7abfdfd5726909a9d738f6f5ed3f09b56d3e Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Tue, 13 Oct 2026 14:31:33 +0200 Subject: [PATCH] Fix privileged RPC username conversion checks Original fix https://github.com/webmin/webmin/commit/616547c26faacae88aacc4798666817f9fa70be0 protected normal user creation, but conversion could still create `root`, `admin`, or `sysadm`. Similarly, conversion now alost skips these usernames unless the admin can edit ACLs. --- acl/convert.cgi | 6 +++++- acl/t/run-tests.t | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/acl/convert.cgi b/acl/convert.cgi index 2e24794ff..936ff0f6a 100755 --- a/acl/convert.cgi +++ b/acl/convert.cgi @@ -108,7 +108,11 @@ foreach my $u (@users) { $msg = "".&text('convert_exists', $u->{'user'}).""; $exists++; } - elsif ($u->{'user'} !~ /^[A-z0-9\-\_\.]+$/) { + elsif ($u->{'user'} !~ /^[A-z0-9\-\_\.]+$/ || + (!$access{'acl'} && + $u->{'user'} =~ /^(root|admin|sysadm)$/)) { + # Reject invalid names and privileged RPC names that this + # administrator is not allowed to create. $msg = "".&text('convert_invalid', $u->{'user'}).""; $invalid++; } diff --git a/acl/t/run-tests.t b/acl/t/run-tests.t index 25b34ed35..e28dbe2fe 100644 --- a/acl/t/run-tests.t +++ b/acl/t/run-tests.t @@ -1474,6 +1474,40 @@ subtest 'save_user.cgi create privilege' => sub { delete_user('admin'); }; +# Unix conversion must not bypass the privileged-name guard in save_user.cgi. +# An ACL-capable administrator can still deliberately convert such a user. +subtest 'convert.cgi privileged username gate' => sub { + _reset_fixture(); + _seed_user_acl('operator', { + sync => 1, create => 1, acl => 0, gassign => '*', + }); + create_group({ name => 'converted', members => [], modules => ['acl'] }); + + my $r = run_cgi('convert.cgi', { + conv => 1, users => 'root', wgroup => 'converted', sync => 1, + }, user => 'operator'); + _clear_caches(); + is($r->{status}, 0, 'conversion completes without ACL access'); + ok(!get_user('root'), + 'conversion skips a privileged RPC username without ACL access'); + like($r->{out}, qr/root.*not a valid Webmin username/i, + 'conversion reports the privileged username as invalid'); + + _reset_fixture(); + _seed_user_acl('operator', { + sync => 1, create => 1, acl => 1, gassign => '*', + }); + create_group({ name => 'converted', members => [], modules => ['acl'] }); + + $r = run_cgi('convert.cgi', { + conv => 1, users => 'root', wgroup => 'converted', sync => 1, + }, user => 'operator'); + _clear_caches(); + is($r->{status}, 0, 'conversion completes with ACL access'); + ok(get_user('root'), + 'ACL-capable administrator can convert a privileged username'); +}; + subtest 'save_user.cgi gassign whitelist' => sub { # gassign='admins' allows assigning only to group "admins"; trying to # assign to a different group should fail.