Harden system calls

Author: drdrew42

lib/LaTeXImage.pm

@@ -271,12 +271,18 @@ sub draw {
 
 sub use_svgMethod {
 	my ($self, $working_dir) = @_;
-	if ($self->svgMethod eq 'dvisvgm') {
-		system WeBWorK::PG::IO::externalCommand('dvisvgm')
-			. " $working_dir/image.dvi --no-fonts --output=$working_dir/image.svg > /dev/null 2>&1";
+
+	# Validate svgMethod against known SVG converters to prevent command injection.
+	my $method = $self->svgMethod;
+	if ($method eq 'dvisvgm') {
+		my $cmd = WeBWorK::PG::IO::externalCommand('dvisvgm');
+		system {$cmd} $cmd, "$working_dir/image.dvi", '--no-fonts', "--output=$working_dir/image.svg";
+	} elsif ($method eq 'pdf2svg') {
+		my $cmd = WeBWorK::PG::IO::externalCommand('pdf2svg');
+		system {$cmd} $cmd, "$working_dir/image.pdf", "$working_dir/image.svg";
 	} else {
-		system WeBWorK::PG::IO::externalCommand($self->svgMethod)
-			. " $working_dir/image.pdf $working_dir/image.svg > /dev/null 2>&1";
+		warn "Unknown svgMethod '$method'. Must be 'dvisvgm' or 'pdf2svg'.";
+		return;
 	}
 	warn "Failed to generate svg file." unless -r "$working_dir/image.svg";
 
@@ -285,11 +291,24 @@ sub use_svgMethod {
 
 sub use_convert {
 	my ($self, $working_dir, $ext) = @_;
-	system WeBWorK::PG::IO::externalCommand('convert')
-		. join('', map { " -$_ " . $self->convertOptions->{input}->{$_} } (keys %{ $self->convertOptions->{input} }))
-		. " $working_dir/image.pdf"
-		. join('', map { " -$_ " . $self->convertOptions->{output}->{$_} } (keys %{ $self->convertOptions->{output} }))
-		. " $working_dir/image.$ext > /dev/null 2>&1";
+
+	# Validate convertOptions keys and values to prevent shell injection.
+	# Only alphanumeric option names and simple values are permitted.
+	my @args;
+	for my $phase (qw(input output)) {
+		my $opts = $self->convertOptions->{$phase} // {};
+		for my $key (keys %$opts) {
+			warn("Invalid convert option name: $key"), next unless $key =~ /^[a-zA-Z][a-zA-Z0-9\-]*$/;
+			my $val = $opts->{$key};
+			warn("Invalid convert option value for $key"), next unless defined $val && $val =~ /^[a-zA-Z0-9.\-+:x%]+$/;
+			push @args, "-$key", $val;
+		}
+		push @args, "$working_dir/image.pdf" if $phase eq 'input';
+	}
+	push @args, "$working_dir/image.$ext";
+
+	my $convert = WeBWorK::PG::IO::externalCommand('convert');
+	system {$convert} $convert, @args;
 	warn "Failed to generate $ext file." unless -r "$working_dir/image.$ext";
 
 	return;

lib/PGalias.pm

@@ -270,7 +270,7 @@ sub convert_file_to_png_for_tex {
 	$self->debug_message('convert filePath ', $sourceFilePath, "\n");
 
 	my $conversion_command = WeBWorK::PG::IO::externalCommand('convert');
-	my $returnCode         = system "$conversion_command '${sourceFilePath}[0]' $targetFilePath";
+	my $returnCode         = system {$conversion_command} $conversion_command, "${sourceFilePath}[0]", $targetFilePath;
 	if ($returnCode || !-e $targetFilePath) {
 		$resource_object->warning_message(
 			qq{Failed to convert "$sourceFilePath" to "$targetFilePath" using "$conversion_command": $!});

lib/WeBWorK/PG/IO.pm

@@ -316,17 +316,40 @@ sub externalCommand {
 # Isolate the call to the sage server in case we have to jazz it up.
 sub query_sage_server {
 	my ($python, $url, $accepted_tos, $setSeed, $webworkfunc, $debug, $curlCommand) = @_;
-	my $sagecall =
-		qq{$curlCommand -i -k -sS -L }
-		. qq{--data-urlencode "accepted_tos=${accepted_tos}" }
-		. qq{--data-urlencode 'user_expressions={"WEBWORK":"_webwork_safe_json(WEBWORK)"}' }
-		. qq{--data-urlencode "code=${setSeed}${webworkfunc}$python" $url};
 
-	my $output = `$sagecall`;
+	# Validate url to prevent shell injection — must look like a URL.
+	if ($url && $url !~ m{^https?://[^\s;|&`\$]+$}) {
+		warn "query_sage_server: invalid url rejected: $url";
+		return;
+	}
+
+	# Use system LIST form via open-pipe to capture output without shell interpolation.
+	my @cmd = (
+		$curlCommand,                                                 '-i',
+		'-k',                                                         '-sS',
+		'-L',                                                         '--data-urlencode',
+		"accepted_tos=$accepted_tos",                                 '--data-urlencode',
+		'user_expressions={"WEBWORK":"_webwork_safe_json(WEBWORK)"}', '--data-urlencode',
+		"code=${setSeed}${webworkfunc}$python",                       $url // (),
+	);
+
 	if ($debug) {
 		warn "debug is turned on in IO.pm. ";
-		warn "\n\nIO::query_sage_server(): SAGE CALL: ", $sagecall, "\n\n";
-		warn "\n\nRETURN from sage call \n",             $output,   "\n\n";
+		warn "\n\nIO::query_sage_server(): SAGE CMD: ", join(' ', @cmd), "\n\n";
+	}
+
+	my $output = '';
+	if (open(my $pipe, '-|', @cmd)) {
+		local $/;
+		$output = <$pipe>;
+		close $pipe;
+	} else {
+		warn "query_sage_server: failed to execute curl: $!";
+		return;
+	}
+
+	if ($debug) {
+		warn "\n\nRETURN from sage call \n", $output, "\n\n";
 		warn "\n\n END SAGE CALL";
 	}