General help

Hmm, this appears a major bug in 1.4.1 (Moodle 1.4.1+ (2004083101)). I just checked in 1.4.1 I get the same thing, but in 1.3.2, I get:

The correct "Sorry, this activity is currently hidden" behavior when trying to navigate to a hidden resource.

Are you using a standard install of Moodle and if so what version?

I think you are right.
I'm now on a testing machine, moodle 1.4.2 nearly (2004083121), and it behaves exactly as you discribe.
Michael Penney is right to say that the bug wasn't in 1.3

Hope it will be fixed soon ...

Has some already filed a bug report ?

We have experienced a similar problem here in York. I have given some thought to this issue and have produced a patch which (I hope) will comprehensively fix the issue.

The issue is potentially much more complicated than suggested here. Some modules may wish to release files only when a condition is met (eg. the exercise module does not display the link to the file when in phase 1 and in this case the URL to file.php should not work either). Also a module (including a resource) could be made visible and later hidden (eg. if it was made visible by mistake) and in this case the correct URL will be known, even if the id is randomised.

I hope you all agree that the best solution would be for file.php to check if access should be granted on each request.

Where a module stores a file within its own directory under the moddata directory a fix is fairly simple. The ownership of the file is clear and all that is needed is a mechanism for file.php to check if the module instance is visible and if so check that access is allowed at the present time.

I propose that a function be defined in the lib.php of each module for file.php to call with the course module object and the filename with a return value of true if access is allowed, false otherwise. If the function does not exist it should be assumed that access is always allowed (to maintain backward compatibility with existing modules). In my patch I have opted to call this function MODULENAME_allow_file_access.

Note that this test occurs after the visibility test; access is automatically denied when a module instance is hidden with no change required to the module code. Note also that the tests are not made if the user is a teacher since teachers can see hidden module instances and can access files directly through the file area anyway.

Where files are stored in the course directory things get more complicated, especially if we are to maintain backward compatibility with old modules. We need to establish ownership of the file (and there may be multiple owners). We need to indicate which module and which instance of that module the file belongs to in the URL so that file.php can check visibility and call MODULENAME_allow_file_access. However we need to verify that the file really is used by that module instance, otherwise access to a hidden resource could be made by simply passing the id for a different resource which is visible.

In order to maintain backward compatibility I propose that files in the course directory and (most) subdirectories are unprotected, i.e. the status quo. However a special directory should be created (which I propose to call .protected as directories starting with a period will be hidden automatically by directory resources) and all files and directories within this will be protected. Access will not be allowed directly to these files by file.php (under any circumstances); instead the URL gives a path within the moddata directory which will be collapsed to the real path. This is easiest to explain by example: 2/moddata/resource/1/.protected/file.ext --> 2/.protected/file.ext with the resource module instance with id 1 asked for permission to serve the file. Note that here the file 2/moddata/resource/1/file.ext does not need to exist and in fact will not be accessible if it does exist as the path will always be collapsed.

Note that this mechanism assumes that nobody uses directories by the name of .protected within their course directories or within a module directory or can easily change (another reason for the name to start with a period).

The protection in my patch is provided by yet another new function in the lib.php for the module. This is passed the course module object and the filename again and is expected to return true if the object belongs to the module instance, false otherwise. Again it is optional but this time defaults to false if not present; thus the student can not change the url to a module which has not been updated to use this mechanism to bypass the check. This is why we can not apply the mechanism to all files but need a special directory. In my patch I call the new function MODULENAME_is_module_file.

Note that old modules will not be able store files in the .protected directory as they will not be accessible and teachers will need to be told which modules do and do not support the security mechanism. Does anybody have any suggestions for what could be done about this? My only suggestion so far is that modules which have been patched should display a message saying "use the .protected dir if you want security" and a message in the file area saying "don't use the .protected directory unless the module tells you that you may". I have not implemented anything yet (we are mainly concerned with making things secure a.s.a.p. here at York).

I have patched the resource module (file and directory types) and exercise module to use my new system. While I was testing the resource module I found a flaw in the directory resource type which I have also fixed in the patch; the moddata directory and any directories starting with a period are removed from the view but the subdir argument is not checked so these directories can still be viewed and navigated once one has correctly guessed the name.

The attached diff is against version 2004083121 (MOODLE_14_STABLE from 8th Oct). Apply in the moodle directory with `patch -p1 < moodle-file-security.diff'. I'll try it against MOODLE_141 and the latest cvs code and report back. I was going to do this first and post later in the week but since this topic has come up...

I should point out that I made the changes against a copy from cvs so the patch contains some of the cvs changes but these are all minor in the files I changed and will do no harm.

Oh, and I couldn't get a less than to work on any formatting setting; everything after it was removed as if it formed a tag and the entity does not seem to be decoded even for html formatting. So please translate the entity manually!