Fix a vulnerability in Statistics.pm and the macro PGstatisticsmacros.pl.

This vulnerability allows a problem to write to any location or
overwrite any file that the server has write permission to.

The problem is that the `write_array_to_CSV` method of the
`Statistics.pm` module takes a file name as its first argument and
writes to that file without question.

To fix this the method instead uses the new `WeBWorK::PG::IO::saveDataToFile`
method which refuses to write anything outside of the html temporary
directory.

Note that the `make_csv_alias` method of the `Statistics.pm` module was
dropped because it uses an invalid approach to creating a unique file
name and is not needed.  So there is no need for the `Statistics`
package to even be an object.  Now there is just the method
`write_array_to_CSV`.

Previously the `insertDataLink` method of `PGstatisticsmacros.pl`
accepted the `$PG` object for its first argument.  That first argument
is still accepted, but is dropped if given, and there is no need to pass
that argument anymore. The macro has direct access to the `$PG` variable
so it is not a needed parameter.

Also, a new optional last argument to the `insertDataLink` is accepted.
If provide this argument must be a reference to a hash, and the
key/value pairs in that hash will be added as attributes to the HTML `a`
tak that is returned. If this parameter is not provided or it is but
does not contain a `download` attribute, then a default `download`
attribute with value `data.csv` will be added.

Note that the `insertDataLink` method is not used by any problem in the
OPL or Contrib.  Furthermore, nothing in the `Statistics.pm` module is
directly used by any problem in OPL or Contrib. Backwards compatibility
is maintained for the `insertDataLink` method for anyone that may have
been using it, but not for the `Statistics.pm` module.  It was not
intended that the module be used directly anyway.

This fixes the second of the three ways that I know of for a problem to
directly write to anywhere that the server has permission to do so.

Author: drgrice1

lib/Statistics.pm

@@ -1,4 +1,3 @@
-
 ################################################################################
 # WeBWorK Online Homework Delivery System
 # Copyright © 2000-2024 The WeBWorK Project, https://github.com/openwebwork
@@ -16,108 +15,36 @@
 
 package Statistics;
 
-#use strict;
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK);
-require Exporter;
-@ISA       = qw(Exporter);
-@EXPORT_OK = qw( );
-@EXPORT    = qw( );
-$VERSION   = '0.01';
-
-# ##########################################
-# Initialize the class
-# Expected argument: a ref to a valid PGcore object
-#
-sub new {
-	my $class = shift;
-	my $self  = { PG => shift };
-
-	bless $self, $class;
-
-}
-
-# ##########################################
-# Create an alias to a csv file.
-# Return the url that can be used in a browser
-# to access the file.
-#
-sub make_csv_alias {
-
-	my $self         = shift;
-	my $studentLogin = shift;
-	my $problemSeed  = shift;
-	my $setname      = shift;
-	my $prob         = shift;
+use strict;
+use warnings;
 
-	# Clean the student login string to make it appropriate for a file name.
-	$studentLogin =~ s/Q/QQ/g;
-	$studentLogin =~ s/\./-Q-/g;
-	$studentLogin =~ s/\,/-Q-/g;
-	$studentLogin =~ s/\@/-Q-/g;
+require WeBWorK::PG::IO;
 
-	# Define the file name, clean it up and convert to a url.
-	my $filePath = "data/$studentLogin-$problemSeed-set" . $setName . "prob$prob.html";
-	$filePath = $self->{PG}->surePathToTmpFile($filePath);
-	my $url = $self->{PG}->{PG_alias}->make_alias($filePath);
-
-	# Remove the .html off the end and replace it with a .csv
-	$filePath =~ s/\.html$/.csv/;
-	$url      =~ s/\.html$/.csv/;
-
-	($filePath, $url);
-}
-
-# ##########################################
 # Write the given data to a csv file.
-# Return the URL if the file is written,
-# an empty string otherwise
-#
 sub write_array_to_CSV {
-	my $self     = shift;
-	my $fileName = shift;
-	my @dataRefs = @_;
+	my ($fileName, @dataRefs) = @_;
+
+	die 'No data set was provided.' unless @dataRefs && ref $dataRefs[0] eq 'ARRAY';
 
 	# Make sure all of the data sets have the same number of elements
-	my $numberDataPoints = "nd";
-	my $data;
-	foreach $data (@dataRefs) {
-		my @dataArray = @{$data};
-		if ($numberDataPoints eq "nd") {
-			$numberDataPoints = $#dataArray;
-		} elsif ($numberDataPoints != $#dataArray) {
-			die("$0", "The number of elements in the data sets are not all the same. No data set written to file.");
-			return;
-		}
+	my $numberDataPoints = $#{ $dataRefs[0] };
+	for my $data (@dataRefs) {
+		die 'The number of elements in the data sets are not all the same. No data set written to file.'
+			if $numberDataPoints != $#$data;
 	}
 
-	# Open the file
-	local (*OUTPUT);    # create local file handle so it won't overwrite other open files.
-	open(OUTPUT, ">$fileName") || warn("$0", "Can't open $fileName<BR>", "");
-	chmod(0777, $filePath);
-
-	#  write the header to the first row.
-	my $header = "";
-	foreach $data (@dataRefs) {
-		$header .= pop(@{$data}) . ",";
-	}
-	$header =~ s/,$//;
-	print OUTPUT ($header . "\n") || warn("$0", "Can't print data file to $fileName<BR>", "");
+	# Add the header to the first row of the output.
+	my $output = join(',', map { $_->[-1] } @dataRefs) . "\n";
 
-	# Go through each data point and write it out.
-	my $lupe;
-	for ($lupe = 0; $lupe < $numberDataPoints; ++$lupe) {
-		my $line = "";
-		foreach $data (@dataRefs) {
-			my @dataSet = @{$data};
-			$line .= $dataSet[$lupe] . ",";
-		}
-		$line =~ s/,$//;
-		print OUTPUT ($line, "\n");
+	# Add each data point as another row in the output.
+	for my $i (0 .. $numberDataPoints - 1) {
+		$output .= join(',', map { $_->[$i] } @dataRefs) . "\n";
 	}
 
-	# Close it up and move on.
-	close(OUTPUT) || warn("$0", "Can't close $filePath<BR>", "");
+	# Write the output to disk.
+	WeBWorK::PG::IO::saveDataToFile($output, $fileName);
 
+	return;
 }
 
 1;

macros/math/PGstatisticsmacros.pl

@@ -787,54 +787,60 @@ sub two_sample_t_test {
 	($t, $df, $p);
 }
 
-=head2 Create a data file and make a link to it.
+=head2 insertDataLink
 
-=pod
+Create a CSV data file and make a link to it.
 
-	Usage: insertDataLink($PG,linkText,@dataRefs)
+Usage: C<insertDataLink($linkText, @dataRefs, $linkAttributes)>
 
-Writes the given data to a file and creates a link to the data file. The string headerTitle is the label used in the anchor link.
-		$PG is a ref to an instance of a PGcore object. (Generally just use $PG in a problem)
-    linkText is the text to appear in the anchor/link.
-    @dataRefs is a list of references. Each reference is assumed to be ref to an array.
-          All of the arrays must have the same length.
-          The last entry in the array is assumed to be the label to use in the first row of the csv file.
+Writes the given data to a CSV file and returns a link to the file.
 
-Usage:
-    # Generate random data
-    @data1 = urand(10.0,2.0,10,2);
-    @data2 = urand(12.0,2.0,10,2);
-    @data3 = urand(14.0,4.0,10,2);
-    @data4 = exprand(0.1,10,2);
+C<$linkText> is the text to appear in the anchor/link.
 
-    # Append the labels for each data set
-    push(@data1,"w");
-    push(@data2,"x");
-    push(@data3,"y");
-    push(@data4,"z");
+C<@dataRefs> is a list of references. Each reference is assumed to be ref to an
+array.  All of the arrays must have the same length.  The last entry in the
+array is assumed to be the label to use in the first row of the csv file.
 
-    BEGIN_TEXT
+C<linkAttributes> is optional.  If provided, this must be a reference to a hash
+containing additional attributes to add to the HTML C<a> tag. If this parameter
+is not provided or it is provided and a C<download> attributed is not specified
+in the hash, then a default C<download> attribute will be added with the value
+"data.csv".
 
-    blah blah
+Usage:
 
-    $BR Data: \{ insertDataLink($PG,"the data",(~~@data1,~~@data2,~~@data3,~~@data4)); \} $BR
+    # Generate random data with labels.
+    $data1 = [ urand(10.0, 2.0, 10, 2), 'w' ];
+    $data2 = [ urand(12.0, 2.0, 10, 2), 'x' ];
+    $data3 = [ urand(14.0, 4.0, 10, 2), 'y' ];
+    $data4 = [ exprand(0.1, 10, 2), 'z' ];
 
+    BEGIN_PGML
+    Data: [@ insertDataLink(
+		'the data', $data1, $data2, $data3, $data4,
+		{ download => 'problem-dataset.csv' }
+	) @]*
+    END_PGML
 
 =cut
 
 sub insertDataLink {
-	my $PG       = shift;
-	my $linkText = shift;
-	my @dataRefs = @_;
-	my $stat     = Statistics->new($PG);
+	my @args = @_;
+
+	# If the $PG object was given then just drop it. This is for backwards compatibility.
+	shift @args if ref($args[0]) eq 'PGcore';
+
+	my ($linkText, @dataRefs) = @args;
+
+	my $linkAttributes = {};
+	$linkAttributes = pop @dataRefs if ref $dataRefs[-1] eq 'HASH';
+	$linkAttributes->{download} = 'data.csv' unless $linkAttributes->{download};
 
-	# Create a file name and get the url as well.
-	my ($fileName, $url) = $stat->make_csv_alias($main::studentLogin, $main::problemSeed, $setName, $main::probNum);
+	my $filePath = $PG->surePathToTmpFile('data/' . $PG->getUniqueName('csv') . '.csv');
 
-	# Now write the data
-	$stat->write_array_to_CSV($fileName, @dataRefs);
+	Statistics::write_array_to_CSV($filePath, @dataRefs);
 
-	"<a href=\"$url\">$linkText</a>";
+	return main::tag('a', href => main::alias($filePath), %$linkAttributes, $linkText);
 }
 
 =head2 Five Point Summary function