From 2e0028cb65a667f9c4c2886e50d0d4347ccef1bf Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Tue, 7 Jul 2026 00:47:22 +0200 Subject: [PATCH] Fix to harden local batch file ACL checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⓘ Fail closed when checking local batch file imports for Webmin-only users that do not have a Unix home directory. This prevents an unset global file root ACL, or a home-relative root such as `~/path`, from being treated as filesystem-root access. Explicitly configured allowed directories still work as before. --- useradmin/t/run-tests.t | 46 +++++++++++++++++++++++++++++++++++++++-- useradmin/user-lib.pl | 13 +++++++++--- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/useradmin/t/run-tests.t b/useradmin/t/run-tests.t index 4cfced6c6..f42ea5d0f 100644 --- a/useradmin/t/run-tests.t +++ b/useradmin/t/run-tests.t @@ -51,12 +51,17 @@ undef(%main::read_file_cache_time); sub write_global_acl { -my (%opts) = @_; +write_acl_for($remote_user, @_); +} + +sub write_acl_for +{ +my ($user, %opts) = @_; my $text = ""; foreach my $k (sort keys %opts) { $text .= "$k=$opts{$k}\n"; } -write_text("$confdir/$remote_user.acl", $text); +write_text("$confdir/$user.acl", $text); clear_acl_cache(); } @@ -85,6 +90,8 @@ subtest 'local batch file path ACLs' => sub { 'outside file is rejected'); ok(!can_read_batch_local_file("relative/batch.txt"), 'relative paths are rejected'); + ok(!can_read_batch_local_file(undef), + 'missing local file path is rejected'); %access = ( 'batchdir' => "" ); ok(!can_read_batch_local_file("$allowed/batch.txt"), @@ -111,4 +118,39 @@ SKIP: { is(read_batch_local_file("$allowed/batch.txt"), "batch\n", 'allowed batch file is read'); +subtest 'global ACL home fallback for Webmin-only users' => sub { + my $webmin_only = "webmin-batch-no-such-user-$$"; + no warnings 'once'; + local $main::remote_user = $webmin_only; + local $main::base_remote_user = $webmin_only; + + write_acl_for( + $webmin_only, + root => "", + otherdirs => "", + fileunix => "nobody", + ); + %access = ( 'batchdir' => "/" ); + ok(!can_read_batch_local_file("$outside/secret.txt"), + 'missing Unix home does not fall back to filesystem root'); + + write_acl_for( + $webmin_only, + root => "~/allowed", + otherdirs => "", + fileunix => "nobody", + ); + ok(!can_read_batch_local_file("/allowed/batch.txt"), + 'home-relative root does not become an absolute path without a home'); + + write_acl_for( + $webmin_only, + root => "", + otherdirs => $allowed, + fileunix => "nobody", + ); + ok(can_read_batch_local_file("$allowed/batch.txt"), + 'explicit otherdirs still permits configured paths'); + }; + done_testing(); diff --git a/useradmin/user-lib.pl b/useradmin/user-lib.pl index d9a2c81cb..a0f703176 100755 --- a/useradmin/user-lib.pl +++ b/useradmin/user-lib.pl @@ -91,7 +91,7 @@ return $data; sub can_read_batch_local_file { my ($file) = @_; -return 0 if ($file !~ /^\//); +return 0 if (!defined($file) || $file !~ /^\//); my $batchdir = defined($access{'batchdir'}) ? $access{'batchdir'} : "/"; return 0 if ($batchdir eq ""); return 0 if (!&is_under_directory($batchdir, $file)); @@ -107,11 +107,18 @@ my %gaccess = &get_module_acl($base_remote_user, ""); my @uinfo = getpwnam($remote_user); my $rootdir; if (!$gaccess{'root'}) { - $rootdir = $uinfo[7] ? $uinfo[7] : "/"; + $rootdir = $uinfo[7] || ""; } else { $rootdir = $gaccess{'root'}; - $rootdir =~ s/^\~/$uinfo[7]/; + if ($rootdir =~ /^\~/) { + if ($uinfo[7]) { + $rootdir =~ s/^\~/$uinfo[7]/; + } + else { + $rootdir = ""; + } + } } foreach my $dir ($rootdir, split(/\t+/, $gaccess{'otherdirs'})) { next if ($dir eq "");