search

at-tools / moodle-block_massaction

The Mass Action block for Moodle 2
6 stars 12 forks source link

duplication/move actions don't check if user has access to original activity. #10

Closed danmarsden closed 8 years ago

danmarsden commented 8 years ago

the action.php file takes a user-populated json encoded var $massaction_request which then is used to obtain a list of course_modules - but these modules don't seem to check if they are all from the same course (it does appear a check is attempted but is done based on user-populated data in $massaction_request) It would probably be nice if there was a check at this point (when populating module_records) to ensure that the user has access to manage this activity.

Where this becomes a bigger problem is in the perform_moveto() and perform_dupto() functions where the permissions to access the original activity are not checked - only the permissions for the new location seem to be checked.

This means that I could probably manually populate the original array and include course_modules from locations that I do not have access to and duplicate or move them into my course.

It would also be really nice if the code made better use of the Moodle coding guidelines to make it easier to review. You could make this easier by adding travis ci support to your plugin as described here: https://moodle.org/mod/forum/discuss.php?d=323384

cdsmith-umn commented 8 years ago

Looking into this.

cdsmith-umn commented 8 years ago

I tried moving/duplicating modules from a course that a non-admin user does not have access to into a course that the same non-admin user does have access to and it failed with an error informing me that I lack the permissions. This happened no matter how I hacked the array or modified the code directly on the server (or combinations thereof). So, if you're able to perform the actions you suspect you'd be able to take on modules you otherwise have no permissions on, please provide instructions to reproduce and I'll be happy to write the fix to prevent it.

I'll see what I can do about getting the code updated to meet modern Moodle standards.