diff --git a/CHANGELOG.md b/CHANGELOG.md index 312f3a9a1..c4871b8c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * Fix two-factor authentication validation to use the Webmin configuration and data directories passed by the caller * Fix HTML escaping of module, script and session fields on the Webmin Actions Log details page * Fix mailbox attachment option injection +* Fix path traversal and command injection in parameterized Custom Commands file editors #### 2.670 (September 20, 2026) * Add new Hardware Information module for inspecting system, firmware, security, PCI, USB, storage, network, processor, sensor, driver, and kernel module details diff --git a/custom/custom-lib.pl b/custom/custom-lib.pl index 7a49e322f..1546d1f9c 100755 --- a/custom/custom-lib.pl +++ b/custom/custom-lib.pl @@ -481,6 +481,11 @@ foreach my $a (@{$cmd->{'args'}}) { &error(&text('run_emust', $a->{'desc'})); } $rv =~ /\0/ && &error($text{'run_ezero'}); + # Stop non-path parameters from changing the editor's directory + if ($cmd->{'edit'} && $a->{'type'} != 5 && $a->{'type'} != 6 && + ($rv =~ /[\/\\\r\n]/ || $rv =~ /^\.\.?$/)) { + &error($text{'view_ecannot'}); + } $ENV{$n} = $rv; $env .= "$n=".quotemeta($rv)."\n"; $export .= " $n"; @@ -496,6 +501,30 @@ foreach my $a (@{$cmd->{'args'}}) { return ($env, $export, $str, $displaystr, \@vals); } +# resolve_editor_file(template) +# Expands environment variables and keeps the path in its fixed directory. +sub resolve_editor_file +{ +local ($file) = @_; +# Leave fixed paths unchanged +return $file if ($file !~ /\$/); +local $template = $file; +# Expand each supported variable once, but not variables inside its value +$file =~ s{\$\{([A-Za-z_][A-Za-z0-9_]*)\}| + \$([A-Za-z_][A-Za-z0-9_]*)}{ + my $name = defined($1) ? $1 : $2; + defined($ENV{$name}) ? $ENV{$name} : ""; + }gex; +# Resolve the fixed directory and full path before checking their locations +$template =~ s/\$.*$//; +$template =~ s/\/[^\/]*$// if ($template ne "/"); +$template = &simplify_path(&resolve_links($template)); +$file = &simplify_path(&resolve_links($file)); +$template && $file && &is_under_directory($template, $file) || + &error($text{'view_ecannot'}); +return $file; +} + # list_dbi_drivers() # Returns a list of DBI driver details, which are actually installed sub list_dbi_drivers diff --git a/custom/save.cgi b/custom/save.cgi index e79b6e255..4588f09b1 100755 --- a/custom/save.cgi +++ b/custom/save.cgi @@ -18,10 +18,9 @@ if ($file !~ /^\//) { # Set environment variables for parameters ($env, $export, $str, $displayfile) = &set_parameter_envs($edit, $file); - if ($edit->{'envs'} || @{$edit->{'args'}}) { - # Do environment variable substitution - chop($file = `echo "$file"`); + # Expand variables without invoking a shell + $file = &resolve_editor_file($file); } # Run the before-command @@ -37,14 +36,15 @@ $in{'data'} =~ s/\r//g; &print_tempfile(FILE, $in{'data'}); &close_tempfile(FILE); -# Set permissions +# Set file ownership and permissions without invoking a shell if ($edit->{'user'}) { - &system_logged("chown $edit->{'user'}:$edit->{'group'} ". - "$file >/dev/null 2>&1"); + # Apply the configured owner and group + &set_ownership_permissions($edit->{'user'}, $edit->{'group'}, + undef, $file); } if ($edit->{'perms'}) { - &system_logged("chmod $edit->{'perms'} ". - "$file >/dev/null 2>&1"); + # Apply the configured file permissions + &set_ownership_permissions(undef, undef, oct($edit->{'perms'}), $file); } # Run the after-command diff --git a/custom/view.cgi b/custom/view.cgi index 54d0e4417..8ea743b2f 100755 --- a/custom/view.cgi +++ b/custom/view.cgi @@ -17,12 +17,11 @@ if ($file !~ /^\//) { # Set environment variables for parameters ($env, $export, $str, $displayfile) = &set_parameter_envs($edit, $file); -$displayfile = &html_escape($displayfile); - if ($edit->{'envs'} || @{$edit->{'args'}}) { - # Do environment variable substitution - chop($file = `echo "$file"`); + # Expand variables without invoking a shell + $file = &resolve_editor_file($file); } +$displayfile = &html_escape($displayfile); # Run any before-edit command if ($edit->{'beforeedit'}) {