Not checking ownership of blocks before editing them

[Aaron Wells]

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.

[Aaron Wells (u-aaronw)]

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.

[Aaron Wells (u-aaronw)]

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().

[Aaron Wells (u-aaronw)]

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

[Aaron Wells (u-aaronw)]

Hi Son,

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

Cheers,
Aaron