General developer forum

Trying to approve plugin. Uploading files to 3rd party service using get_file_by_hash from plagiarism plugin.

 
Picture of Eli Rabinovitz
Trying to approve plugin. Uploading files to 3rd party service using get_file_by_hash from plagiarism plugin.
 

Hi,

We are developing a plagiarism plugin. we show links in the submitted assignment list. Each link we show has a hash as a property from $file->get_pathnamehash() . We get the $file object through the public function get_links in our lib.php.

We then pass this pathnamehash to another php file, where we get the file again with $file = $fs->get_file_by_hash($pathnamehash);

Finally we upload this file to our server ( the 3rd party ) using moodle curl.

Our plugin is being rejected because of this usage. Could I please get some help understanding what we are doing wrong. And what alternatives there are?

 Thanks, Eli.

 
Average of ratings: -
Picture of Dan Marsden
Re: Trying to approve plugin. Uploading files to 3rd party service using get_file_by_hash from plagiarism plugin.
Core developersMoodle Course Creator Certificate holdersParticularly helpful MoodlersPlugin developersPlugins guardiansTestersTranslators

To clarify for others reading this - (Eli - you might want to link to your code to show what is going on)

1) The code allows the user to pass a contenthash param and allows the logged in user to access "ANY" file in the system by passing a different contenthash - this violates Moodle coding guidelines and presents some security issues. Before serving or accessing the file, correct capability checking should be in place to verify that the user should be able to access this particular file. Fixing this properly will require you to rewrite the way you access the files and pass them to your plagiarism service.

2) The function plagiarism_copyleaks_file_hash_name_to_path takes the same param and is vulnerable to a path traversal attack.

You should take a look at the other plagiarism plugins to see how they send files to the external service - if you post a link to your code other community members may provide further feedback.

thanks!

 
Average of ratings: -
Picture of Eli Rabinovitz
Re: Trying to approve plugin. Uploading files to 3rd party service using get_file_by_hash from plagiarism plugin.
 
Thanks Dan, good idea.

Here is the code:

https://github.com/Copyleaks/moodle-plagiarism_copyleaks/blob/master/lib.php:

$scanurl = new moodle_url('/plagiarism/copyleaks/scan.php',

            array('pathnamehash' => $file->get_pathnamehash(),

                  'cmid' => $cmid));

        $output = html_writer::div(

            html_writer::link($scanurl, html_writer::tag('span',

                html_writer::empty_tag('img',

                    array('src' => $CFG->wwwroot . '/plagiarism/copyleaks/pix/small-logo.png', 'alt' => 'Copyleaks logo')) .

                get_string('copyleaks_report', 'plagiarism_copyleaks'))

                , array('target' => '_blank')));

        return $output;


And https://github.com/Copyleaks/moodle-plagiarism_copyleaks/blob/master/scan.php,

$pathnamehash = required_param('pathnamehash', PARAM_TEXT);

$fs = get_file_storage();

$file = $fs->get_file_by_hash($pathnamehash);


For broader context, The code can be found in the following 2 files:

  1. https://github.com/Copyleaks/moodle-plagiarism_copyleaks/blob/master/lib.php ( lines 232-242 )
  2. https://github.com/Copyleaks/moodle-plagiarism_copyleaks/blob/master/scan.php ( lines 60 - 61 )


So if I understand correctly, to fix this, there is a set of parameters that can point to a moodle file and are safe to pass between pages so that the file can be served and its permissions be checked?

Thanks, Eli.

 
Average of ratings: -