diff --git a/CHANGELOG.md b/CHANGELOG.md index 62ab467bb..ddd36f575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Fix to recognize hex numeric HTML entities to work in various elements * Fix `patch` sub-command to reload Webmin instead of restarting * Fix SSL certificate and TCP monitors to report transient connection failures as down, and SSL check timeouts as timed out, rather than uninstalled +* Fix local batch file imports to enforce file access ACLs in Users and Groups module #### 2.651 (June 28, 2026) * Fix Certbot-backed certificate requests and renewals to correctly parse PEM paths after issuance diff --git a/useradmin/batch_exec.cgi b/useradmin/batch_exec.cgi index e15d0c6f6..49e61184f 100755 --- a/useradmin/batch_exec.cgi +++ b/useradmin/batch_exec.cgi @@ -15,13 +15,7 @@ if ($in{'source'} == 0) { $data =~ /\S/ || &error($text{'batch_efile'}); } elsif ($in{'source'} == 1) { - &is_under_directory($access{'batchdir'}, $in{'local'}) || - &error($text{'batch_elocaldir'}); - open(LOCAL, "<$in{'local'}") || &error($text{'batch_elocal'}); - while() { - $data .= $_; - } - close(LOCAL); + $data = &read_batch_local_file($in{'local'}); } elsif ($in{'source'} == 2) { $data = $in{'text'}; @@ -669,4 +663,3 @@ foreach my $f ('min', 'max', 'warn', 'inactive', 'expire', 'change') { } return undef; } - diff --git a/useradmin/gbatch_exec.cgi b/useradmin/gbatch_exec.cgi index 9a45ebda0..bd36447d2 100755 --- a/useradmin/gbatch_exec.cgi +++ b/useradmin/gbatch_exec.cgi @@ -14,13 +14,7 @@ if ($in{'source'} == 0) { $data =~ /\S/ || &error($text{'batch_efile'}); } elsif ($in{'source'} == 1) { - &is_under_directory($access{'batchdir'}, $in{'local'}) || - &error($text{'batch_elocaldir'}); - open(LOCAL, "<$in{'local'}") || &error($text{'batch_elocal'}); - while() { - $data .= $_; - } - close(LOCAL); + $data = &read_batch_local_file($in{'local'}); } elsif ($in{'source'} == 2) { $data = $in{'text'}; @@ -333,4 +327,3 @@ if ($_[1] && !$access{'ggid'} && $_[1]->{'gid'} != $_[0]->{'gid'}) { } return undef; } - diff --git a/useradmin/t/run-tests.t b/useradmin/t/run-tests.t new file mode 100644 index 000000000..4cfced6c6 --- /dev/null +++ b/useradmin/t/run-tests.t @@ -0,0 +1,114 @@ +#!/usr/bin/perl +use strict; +use warnings; +use Test::More; +use Cwd qw(abs_path); +use File::Basename qw(dirname); +use File::Path qw(make_path); +use File::Temp qw(tempdir); + +my $bindir = dirname(__FILE__); +my $rootdir = abs_path("$bindir/../..") or die "rootdir: $!"; +my $confdir = tempdir(CLEANUP => 1); +my $vardir = tempdir(CLEANUP => 1); +my $remote_user = getpwuid($<) || 'root'; + +sub write_text +{ +my ($file, $text) = @_; +open(my $fh, ">", $file) or die "write $file: $!"; +print $fh $text; +close($fh) or die "close $file: $!"; +} + +write_text("$confdir/config", "os_type=generic-linux\nos_version=0\n"); +write_text("$confdir/var-path", "$vardir\n"); +write_text("$confdir/miniserv.conf", + "root=$rootdir\nuserfile=$confdir/miniserv.users\n"); +write_text("$confdir/miniserv.users", ""); +make_path("$confdir/useradmin"); + +$ENV{'WEBMIN_CONFIG'} = $confdir; +$ENV{'WEBMIN_VAR'} = $vardir; +$ENV{'MINISERV_CONFIG'} = "$confdir/miniserv.conf"; +$ENV{'FOREIGN_MODULE_NAME'} = 'useradmin'; +$ENV{'FOREIGN_ROOT_DIRECTORY'} = $rootdir; +$ENV{'REMOTE_USER'} = $remote_user; +$ENV{'BASE_REMOTE_USER'} = $remote_user; + +chdir("$rootdir/useradmin") or die "chdir useradmin: $!"; +require "$rootdir/useradmin/user-lib.pl"; + +our %access; + +sub clear_acl_cache +{ +no warnings 'once'; +undef(%main::read_file_cache); +undef(%main::read_file_missing); +undef(%main::read_file_cache_time); +} + +sub write_global_acl +{ +my (%opts) = @_; +my $text = ""; +foreach my $k (sort keys %opts) { + $text .= "$k=$opts{$k}\n"; + } +write_text("$confdir/$remote_user.acl", $text); +clear_acl_cache(); +} + +my $tmp = tempdir(CLEANUP => 1); +my $allowed = "$tmp/allowed"; +my $other = "$tmp/other"; +my $outside = "$tmp/outside"; +make_path($allowed, $other, $outside); +write_text("$allowed/batch.txt", "batch\n"); +write_text("$other/also-batch.txt", "other\n"); +write_text("$outside/secret.txt", "secret\n"); + +write_global_acl( + root => $allowed, + otherdirs => $other, + fileunix => $remote_user, + ); + +subtest 'local batch file path ACLs' => sub { + %access = ( 'batchdir' => $allowed ); + ok(can_read_batch_local_file("$allowed/batch.txt"), + 'file under batchdir and global root is allowed'); + ok(!can_read_batch_local_file("$other/also-batch.txt"), + 'global otherdirs alone cannot bypass narrower batchdir'); + ok(!can_read_batch_local_file("$outside/secret.txt"), + 'outside file is rejected'); + ok(!can_read_batch_local_file("relative/batch.txt"), + 'relative paths are rejected'); + + %access = ( 'batchdir' => "" ); + ok(!can_read_batch_local_file("$allowed/batch.txt"), + 'blank batchdir denies local server files'); + + %access = ( ); + ok(can_read_batch_local_file("$allowed/batch.txt"), + 'missing batchdir falls back to / for old ACL compatibility'); + + %access = ( 'batchdir' => $tmp ); + ok(can_read_batch_local_file("$other/also-batch.txt"), + 'batchdir plus global otherdirs allows second configured tree'); + }; + +SKIP: { + skip('symlinks are unavailable', 1) + if !symlink("$outside/secret.txt", "$allowed/link.txt"); + %access = ( 'batchdir' => $allowed ); + ok(!can_read_batch_local_file("$allowed/link.txt"), + 'symlink escaping batchdir/global root is rejected'); + } + +%access = ( 'batchdir' => $allowed ); +is(read_batch_local_file("$allowed/batch.txt"), "batch\n", + 'allowed batch file is read'); + +done_testing(); diff --git a/useradmin/user-lib.pl b/useradmin/user-lib.pl index 6a43e4faf..d9a2c81cb 100755 --- a/useradmin/user-lib.pl +++ b/useradmin/user-lib.pl @@ -63,6 +63,77 @@ $password_file_cache{$file} = $rv; return $rv; } +# read_batch_local_file(file) +# Returns the contents of a server-side batch file, after checking both the +# Users and Groups batch directory ACL and the global file chooser ACL. +sub read_batch_local_file +{ +my ($file) = @_; +&can_read_batch_local_file($file) || &error($text{'batch_elocaldir'}); +my $data; +my ($fileunix, $fileunix_err) = &batch_acl_fileunix(); +if ($fileunix_err) { + &error($text{'batch_elocal'}); + } +if ($fileunix && &supports_users() && $< == 0) { + $data = &eval_as_unix_user($fileunix, + sub { return &read_file_contents($file); }); + } +else { + $data = &read_file_contents($file); + } +defined($data) || &error($text{'batch_elocal'}); +return $data; +} + +# can_read_batch_local_file(file) +# Returns 1 if a local batch file path is allowed by all relevant ACLs. +sub can_read_batch_local_file +{ +my ($file) = @_; +return 0 if ($file !~ /^\//); +my $batchdir = defined($access{'batchdir'}) ? $access{'batchdir'} : "/"; +return 0 if ($batchdir eq ""); +return 0 if (!&is_under_directory($batchdir, $file)); +return &is_under_global_file_acl($file); +} + +# is_under_global_file_acl(file) +# Returns 1 if a file is under the Webmin user's global file chooser ACL. +sub is_under_global_file_acl +{ +my ($file) = @_; +my %gaccess = &get_module_acl($base_remote_user, ""); +my @uinfo = getpwnam($remote_user); +my $rootdir; +if (!$gaccess{'root'}) { + $rootdir = $uinfo[7] ? $uinfo[7] : "/"; + } +else { + $rootdir = $gaccess{'root'}; + $rootdir =~ s/^\~/$uinfo[7]/; + } +foreach my $dir ($rootdir, split(/\t+/, $gaccess{'otherdirs'})) { + next if ($dir eq ""); + return 1 if (&is_under_directory($dir, $file)); + } +return 0; +} + +# batch_acl_fileunix() +# Returns the Unix user to read local batch files as, or an error flag. +sub batch_acl_fileunix +{ +my %gaccess = &get_module_acl($base_remote_user, ""); +my $fileunix = $gaccess{'fileunix'} || $remote_user; +my @uinfo = getpwnam($fileunix); +if (!@uinfo && !$gaccess{'fileunix'}) { + $fileunix = "nobody"; + @uinfo = getpwnam($fileunix); + } +return @uinfo ? ($fileunix, 0) : (undef, 1); +} + =head2 list_users Returns an array of hash references, each containing info about one user. Each