Not checking ownership of blocks before editing them

Bug #1233500 reported by Aaron Wells on 2013-10-01
256
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
High
Son Nguyen
1.5
Undecided
Unassigned
1.6
Undecided
Unassigned
1.7
Undecided
Unassigned
mahara (Debian)
Fix Released
Unknown

Bug Description

While working on issue https://bugs.launchpad.net/mahara/+bug/1211758 , I noticed that I could spoof the ID of the block that I wanted to edit, and by doing this I could edit other users' blocks. I used the "Burp Suite" tool to edit HTTP requests between my browser and my web server.

Steps:
1. Create a Mahara site with two users, A and B
2. User A creates a page with a text block that has ID 35
3. User B creates a page with a text block that has ID 105
4. User B edits their text block, ID 105
5. User B doctors the HTTP request so that the block ID in it is "35" instead of "105"

Result: User A's block 35 has all of its contents overwritten by the settings for block 105.

This attack could be done either by serially guessing IDs, or possibly by getting the ID by looking at a page that the user has view access to.

CVE References

Aaron Wells (u-aaronw) wrote :

There are several different places in the code where it might be appropriate to check for Block ownership. BlockInstance->instance_config_store() is one possibility (though it could be overridden by child blocks).

We check View ownership in blocks.php, so it might make sense to check the block ownership there as well.

Or, in View->process_changes(), before we set $this->blockinstance_currently_being_configured, it might make sense to verify at that point that the block ID is a block that actually belongs to this view.

Changed in mahara:
assignee: nobody → Robert Lyon (robertl-9)
Aaron Wells (u-aaronw) wrote :

Similarly, you can also delete another user's block. And I suspect you can probably move one as well.

The most appropriate place to check for block ownership before deleting or moving, because of how they pass through View->process_changes(), is probably in the functions View->moveblockinstance() and View->removeblockinstance().

Son Nguyen (ngson2000) on 2013-10-01
Changed in mahara:
assignee: Robert Lyon (robertl-9) → Son Nguyen (ngson2000)
Aaron Wells (u-aaronw) wrote :

Son posted this patch for the issue: https://reviews.mahara.org/#/c/2563

Aaron Wells (u-aaronw) wrote :

Hi Son,

I approved and merged your commit 2563. Can you please backport it to 1.5 - 1.7 ?

Cheers,
Aaron

Aaron Wells (u-aaronw) on 2013-10-03
information type: Private Security → Public Security
Aaron Wells (u-aaronw) on 2013-10-24
no longer affects: mahara/1.8
Changed in mahara:
status: Fix Committed → Fix Released
affects: debian → mahara (Debian)
Changed in mahara (Debian):
status: Unknown → Confirmed
Changed in mahara (Debian):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.