Fix Custom Commands editor path injection

This fixes path traversal and command injection in Custom Commands file editors.

It replaces shell-based path expansion with confined Perl expansion and validates non-path parameters.

It also applies ownership and permissions without invoking a shell.

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

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

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

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'}) {