Moodle

portfolio export does not verify activity access control

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Portfolio API
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

Portfolio export needs to verify access control, the easiest way is to use require_login() with correct $cm parameter - replicating the logic from require_login() would be probably a major maintenance problem...

Sample exploit:
1/ go to forum in one browser and copy "Save..." link
2/ make module hidden in another browser as where you are logged in as admin
3/ paste the url in first browser - export will complete anyway

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Penny Leach added a comment -

Interesting, each of the portfolio_caller subclasses needs to implement check_permissions function, which usually does a has_capability check. See mod/forum/lib.php, search for check_permissions function.

Maybe it's not being called properly when the exporter object is rewoken across requests. I'll check that.

I don't agree with your best fix though. Just doing require_login is not enough, we have to check the export capabilities on the context being exported as well.

Show
Penny Leach added a comment - Interesting, each of the portfolio_caller subclasses needs to implement check_permissions function, which usually does a has_capability check. See mod/forum/lib.php, search for check_permissions function. Maybe it's not being called properly when the exporter object is rewoken across requests. I'll check that. I don't agree with your best fix though. Just doing require_login is not enough, we have to check the export capabilities on the context being exported as well.
Hide
Permalink
Penny Leach added a comment -

I added $this->caller->check_permissions to the verify_rewaken method in the portfolio_exporter object. This ensures the caller permissions are checked every page request again on the active export object.

If you're happy with that fix let's close this bug now.

Show
Penny Leach added a comment - I added $this->caller->check_permissions to the verify_rewaken method in the portfolio_exporter object. This ensures the caller permissions are checked every page request again on the active export object. If you're happy with that fix let's close this bug now.
Hide
Permalink
Petr Škoda (skodak) added a comment -

I do not understand, I did not say to use only require_login() - both has_capability() and require_login($course, false, $cm) must be used before allowing any export from activities

Show
Petr Škoda (skodak) added a comment - I do not understand, I did not say to use only require_login() - both has_capability() and require_login($course, false, $cm) must be used before allowing any export from activities
Hide
Permalink
Penny Leach added a comment -

They are both checked before starting export.

Your exploit was about the situation changing midway through the export and the permissions not being verified between 2 and 3 in your example above, which is what i've fixed in my last commit.

I'm not sure about the $cm parameter to require_login but that's a separate issue to check, which I will do now.

Show
Penny Leach added a comment - They are both checked before starting export. Your exploit was about the situation changing midway through the export and the permissions not being verified between 2 and 3 in your example above, which is what i've fixed in my last commit. I'm not sure about the $cm parameter to require_login but that's a separate issue to check, which I will do now.
Hide
Permalink
Penny Leach added a comment -

added $cm and a todo about changing this whenever portfolio export gets implemented somewhere other than a cm (blog, for instance)

Show
Penny Leach added a comment - added $cm and a todo about changing this whenever portfolio export gets implemented somewhere other than a cm (blog, for instance)

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.