Around line 77 of file.php, where it says:

// security: only editing teachers can access backups
if (!has_capability('moodle/site:backup', ...

And an "or has_capability('moodle/question:view', ...". This will let people download question export files they have created. However, we will not change file/index.php, so that still only people with moodle/site:backup will be able to browse that folder.

This is not properly secure because it will let people with moodle/question:view download course backups, if they can guess the file name. However, those file names are hard to guess.

I was going to say that I did not think this was a very big security risk, because file accesses are logged, so if someone is naughty, we can catch them. However, now I look, I find that they are not logged. Weird. Sam wrote some code to log file accesses in our code. I told him to put a patch in the tracker.

Of course, in Moodle 2.0, we will be able to fix this securely with the repository API, using the access control lists it provides, but I think it is important to get this fixed in 1.9.x

Petr, please can you review this suggestion with your security hat on. Thanks.

Ciao

I think there should be a different folder for this

• we no not have to care about security much
• no wasting of space in moodledata
• simple use in various export scripts

get_temp_file_name($mod, $userid = 0) {\
global $CFG, $USER;
if (!$userid) {
$userid = $USER->id;
}
$folder = "$CFG->dataroot/temp/export/$mod/$userid";
ensure_directory_exists($folder);
return tempnam($folder, '');

(Sorry can't remember if our function is actually called ensure_directory_exists.)

This whole scheme will leave lots of empty directories lying around?

It does not change behaviour for people with 'moodle/site:backup' capability. For people without it, it saves the export file in a temporary area, and automatically starts the download using JavaScript with a fallback link, via a new question/exportfile.php stript.

Some security is provided because the temp file name contains the $USER->id, but the question/exportfile.php URL does not.

If this does go in to 1.9.2, I will merge it to head, and file a new track issue to re-write this bit of code using the file API, once that is done.

thanks