Fix to tighten privileged ACL checks

This PR addresses privately reported issue and tries to restricts root/UID 0 user mutations and Custom Commands definition editing to full, non-safe Webmin admins.

This closes privilege-escalation paths for restricted operators while preserving execution of pre-approved Custom Commands under existing cmds ACLs.

custom/cgi_args.pl

@@ -7,9 +7,9 @@ my ($cgi) = @_;
 my @cust = grep { &can_run_command($_) } &list_commands();
 if ($cgi eq 'edit_cmd.cgi') {
 	# Custom command editor
+	return 'none' if (!&custom_can_edit_commands());
 	my ($cmd) = grep { !$_->{'edit'} && !$_->{'sql'} } @cust;
-	return $cmd ? 'id='.&urlize($cmd->{'id'}) :
-	       $access{'edit'} ? 'new=1' : 'none';
+	return $cmd ? 'id='.&urlize($cmd->{'id'}) : 'new=1';
 	}
 elsif ($cgi eq 'form.cgi') {
 	# Custom command form
@@ -18,9 +18,9 @@ elsif ($cgi eq 'form.cgi') {
 	}
 elsif ($cgi eq 'edit_file.cgi') {
 	# File editor editor
+	return 'none' if (!&custom_can_edit_commands());
 	my ($cmd) = grep { $_->{'edit'} } @cust;
-	return $cmd ? 'id='.&urlize($cmd->{'id'}) :
-	       $access{'edit'} ? 'new=1' : 'none';
+	return $cmd ? 'id='.&urlize($cmd->{'id'}) : 'new=1';
 	}
 elsif ($cgi eq 'view.cgi') {
 	# Custom command form
@@ -29,9 +29,9 @@ elsif ($cgi eq 'view.cgi') {
 	}
 elsif ($cgi eq 'edit_sql.cgi') {
 	# SQL query
+	return 'none' if (!&custom_can_edit_commands());
 	my ($cmd) = grep { $_->{'sql'} } @cust;
-	return $cmd ? 'id='.&urlize($cmd->{'id'}) :
-	       $access{'edit'} ? 'new=1' : 'none';
+	return $cmd ? 'id='.&urlize($cmd->{'id'}) : 'new=1';
 	}
 elsif ($cgi eq 'sqlform.cgi') {
 	# SQL query form

custom/custom-lib.pl

@@ -6,6 +6,34 @@ use WebminCore;
 &init_config();
 %access = &get_module_acl();
 
+# custom_has_full_webmin_access()
+# Returns 1 if the current Webmin user has access to all modules.
+sub custom_has_full_webmin_access
+{
+return $custom_full_webmin_access_cache
+	if (defined($custom_full_webmin_access_cache));
+local %acl;
+&read_acl(\%acl, undef, [ $base_remote_user ]);
+local %global_access = &get_module_acl($base_remote_user, "");
+return $custom_full_webmin_access_cache = 0
+	if ($global_access{'_safe'} || $global_access{'rpc'} == 0);
+return $custom_full_webmin_access_cache = 1
+	if ($acl{$base_remote_user,'*'});
+foreach my $m (&get_all_module_infos()) {
+	next if (!&check_os_support($m));
+	return $custom_full_webmin_access_cache = 0
+		if (!$acl{$base_remote_user,$m->{'dir'}});
+	}
+return $custom_full_webmin_access_cache = 1;
+}
+
+# custom_can_edit_commands()
+# Returns 1 if the current Webmin user can create or edit commands.
+sub custom_can_edit_commands
+{
+return $access{'edit'} && &custom_has_full_webmin_access();
+}
+
 # list_commands()
 # Returns a list of all custom commands
 sub list_commands

custom/edit_cmd.cgi

@@ -5,7 +5,7 @@
 require './custom-lib.pl';
 &ReadParse();
 
-$access{'edit'} || &error($text{'edit_ecannot'});
+&custom_can_edit_commands() || &error($text{'edit_ecannot'});
 if ($in{'new'}) {
 	&ui_print_header(undef, $text{'create_title'}, "", "create");
 	if ($in{'clone'}) {

custom/edit_file.cgi

@@ -5,7 +5,7 @@
 require './custom-lib.pl';
 &ReadParse();
 
-$access{'edit'} || &error($text{'file_ecannot'});
+&custom_can_edit_commands() || &error($text{'file_ecannot'});
 if ($in{'new'}) {
 	&ui_print_header(undef, $text{'fcreate_title'}, "", "fcreate");
 	if ($in{'clone'}) {

custom/edit_sql.cgi

@@ -20,7 +20,7 @@ if (!@drivers) {
 		"../cpan/download.cgi?source=3&cpan=$pgneed&return=/$module_name/&returndesc=".&urlize($text{'index_return'})),"<p>\n";
 	}
 
-$access{'edit'} || &error($text{'edit_ecannot'});
+&custom_can_edit_commands() || &error($text{'edit_ecannot'});
 if ($in{'new'}) {
 	&ui_print_header(undef, $text{'sql_title1'}, "");
 	if ($in{'clone'}) {

custom/index.cgi

@@ -10,7 +10,7 @@ require './custom-lib.pl';
 
 # Build links
 @links = ( );
-if ($access{'edit'}) {
+if (&custom_can_edit_commands()) {
 	push(@links,&ui_link("edit_cmd.cgi?new=1",$text{'index_create'}));
 	push(@links,&ui_link("edit_file.cgi?new=1",$text{'index_ecreate'}));
 	push(@links,&ui_link("edit_sql.cgi?new=1",$text{'index_screate'}));
@@ -66,7 +66,7 @@ elsif ($config{'display_mode'} == 0) {
 			$html .= &ui_table_row(&html_escape($a->{'desc'}),
 					&show_parameter_input($a, $formno));
 			}
-		if ($access{'edit'}) {
+		if (&custom_can_edit_commands()) {
 			if ($c->{'edit'}) {
 				$link = &ui_link("edit_file.cgi?id=$c->{'id'}",$text{'index_fedit'});
 				}
@@ -98,7 +98,7 @@ else {
 	foreach $c (@cust) {
 		@cols = ( );
 		local @links = ( );
-		if ($access{'edit'}) {
+		if (&custom_can_edit_commands()) {
 			local $e = $c->{'edit'} ? "edit_file.cgi" :
 				   $c->{'sql'} ? "edit_sql.cgi" :
 						 "edit_cmd.cgi";

custom/save_cmd.cgi

@@ -5,7 +5,7 @@
 require './custom-lib.pl';
 &ReadParse();
 
-$access{'edit'} || &error($text{'save_ecannot'});
+&custom_can_edit_commands() || &error($text{'save_ecannot'});
 if ($in{'delete'}) {
 	$cmd = &get_command($in{'id'}, $in{'idx'});
 	&delete_command($cmd);

custom/save_file.cgi

@@ -5,7 +5,7 @@
 require './custom-lib.pl';
 &ReadParse();
 
-$access{'edit'} || &error($text{'file_ecannot'});
+&custom_can_edit_commands() || &error($text{'file_ecannot'});
 if ($in{'delete'}) {
 	$edit = &get_command($in{'id'}, $in{'idx'});
 	&delete_command($edit);

custom/save_sql.cgi

@@ -4,7 +4,7 @@
 require './custom-lib.pl';
 &ReadParse();
 
-$access{'edit'} || &error($text{'save_ecannot'});
+&custom_can_edit_commands() || &error($text{'save_ecannot'});
 if ($in{'delete'}) {
 	$cmd = &get_command($in{'id'}, $in{'idx'});
 	&delete_command($cmd);

useradmin/batch_exec.cgi

@@ -52,6 +52,7 @@ $newgid = int($config{'base_gid'} > $access{'lowgid'} ?
 # Process the file
 &batch_start() if ($in{'batch'});
 &lock_user_files();
+$full_webmin_access = &useradmin_has_full_webmin_access();
 $lnum = $created = $modified = $deleted = 0;
 print "<pre>\n";
 $pft = &passfiles_type();
@@ -601,6 +602,15 @@ print "</pre>\n";
 # Check access control restrictions for a user
 sub check_user
 {
+if (!$full_webmin_access) {
+	if ($_[0]->{'user'} eq 'root') {
+		return $text{'usave_eedit'};
+		}
+	if ($_[0]->{'uid'} <= 0) {
+		return &text('usave_elowuid', 1);
+		}
+	}
+
 # check if uid is within range
 if ($access{'lowuid'} && $_[0]->{'uid'} < $access{'lowuid'}) {
 	return &text('usave_elowuid', $access{'lowuid'});

useradmin/save_user.cgi

@@ -58,6 +58,7 @@ $err = &check_username_restrictions($in{'user'});
 &lock_user_files();
 @ulist = &list_users();
 @glist = &list_groups();
+$full_webmin_access = &useradmin_has_full_webmin_access();
 if ($in{'old'} ne "") {
 	# Get old user info
 	($ouser_hash) = grep { $_->{'user'} eq $in{'old'} } @ulist;
@@ -151,6 +152,12 @@ elsif ( $in{'uid_def'} eq '2' ) {
 		}
 	}
 
+if (!$full_webmin_access && $in{'user'} eq 'root') {
+	&error($text{'usave_eedit'});
+	}
+if (!$full_webmin_access && $in{'uid'} <= 0) {
+	&error(&text('usave_elowuid', 1));
+	}
 $in{'real'} =~ /^[^:]*$/ || &error(&text('usave_ereal', $in{'real'}));
 if ($in{'shell'} eq "*") { $in{'shell'} = $in{'othersh'}; }
 if ($access{'shells'} ne "*") {

useradmin/user-lib.pl

@@ -30,6 +30,27 @@ do "md5-lib.pl";
 @random_password_chars = ( 'a' .. 'z', 'A' .. 'Z', '0' .. '9' );
 $disable_string = $config{'lock_prepend'} eq "" ? "!" : $config{'lock_prepend'};
 
+# useradmin_has_full_webmin_access()
+# Returns 1 if the current Webmin user has access to all modules.
+sub useradmin_has_full_webmin_access
+{
+return $useradmin_full_webmin_access_cache
+	if (defined($useradmin_full_webmin_access_cache));
+local %acl;
+&read_acl(\%acl, undef, [ $base_remote_user ]);
+local %global_access = &get_module_acl($base_remote_user, "");
+return $useradmin_full_webmin_access_cache = 0
+	if ($global_access{'_safe'} || $global_access{'rpc'} == 0);
+return $useradmin_full_webmin_access_cache = 1
+	if ($acl{$base_remote_user,'*'});
+foreach my $m (&get_all_module_infos()) {
+	next if (!&check_os_support($m));
+	return $useradmin_full_webmin_access_cache = 0
+		if (!$acl{$base_remote_user,$m->{'dir'}});
+	}
+return $useradmin_full_webmin_access_cache = 1;
+}
+
 # Search types
 $match_modes = [ [ 0, $text{'index_equals'} ], [ 4, $text{'index_contains'} ],
 		 [ 1, $text{'index_matches'} ], [ 2, $text{'index_nequals'} ],
@@ -1004,6 +1025,11 @@ control permissions for this module are in the acl parameter.
 =cut
 sub can_edit_user
 {
+if (!&useradmin_has_full_webmin_access() && defined($_[1]->{'user'}) &&
+    ($_[1]->{'user'} eq 'root' ||
+     (defined($_[1]->{'uid'}) && $_[1]->{'uid'} <= 0))) {
+	return 0;
+	}
 local $m = $_[0]->{'uedit_mode'};
 local %u;
 if ($m == 0) { return 1; }