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

[drgrice1]

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 added in #1219 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.

[drgrice1]

Attached are two problems to test this with.
PGstatisticsmacrosTests.zip

The pgstatistics-write-csv.pg problem can be used to test that things work with develop or main and with this pull request. You will need to comment out or delete the second part in the BEGIN_PGML/END_PGML section to test with develop or main though.

The pg-statistics-vulnerability.pg file demonstrates the vulnerability. It will write a badfile.csv file to the courses directory. With minor modification it would write any file with whatever contents are desired though.

[drgrice1]

By the way, don't expect an immediate pull request for the third way that I know of that a problem can write a file wherever it wants to that the server has write permissions for. That one will take quite a bit more work.

[pstaabp]

I see the second problem poses a problem and this fixes that.

Just to clarify, Is the second problem as an example of using the insertDataLink without the $PG passing as the first argument?

[drgrice1]

The pgstatistics-write-csv.pg problem is an example of using the insertDataLink method both with and without passing $PG as the first argument. In the BEGIN_PGML/END_PGML section of that problem there are two calls to insertDataLink. The first passes the $PG parameter, and the second doesn't. It does not demonstrate the vulnerability. The other problem does.

[pstaabp]

Relooking after the other PR went it. Looks good.