560466 - Arbitrary code execution using userdata handlers

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[moz_bug_r_a4]

Since we call userdata handlers without dealing with JS context stuff, if a
chrome code manipulates a content node and a userdata handler is called, the
userdata handler's scripted caller is the chrome code.

When viewPartialSourceForSelection() calls ancestorContainer.cloneNode(),
content can run arbitrary code with chrome privileges.

Comment 1

[moz_bug_r_a4]

Attachment: testcase

This uses bug 344495's trick.  This depends on bug 554449.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

blake, any chance to get rid of the manual context pushing at something point?

Which way does this depend on bug 554449? Does fixing it fix also this one?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Or does the testcase depend on bug 554449?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

But for now, I'm going to do whatever is done with event listeners.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 6

[moz_bug_r_a4]

(In reply to comment #2)
> Which way does this depend on bug 554449? Does fixing it fix also this one?

Fixing bug 554449 fixes the testcase.  If bug 554449 is fixed, when a content
function is called, the subject principal is the content principal even if the
scripted caller is the chrome code.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Ok, that sounds good. I wonder if we could remove some context pushing after
bug 554449 is fixed. Blake?

Comment 8

[Peter Van der Beken [:peterv]]

Comment on attachment 440179 [details] [diff] [review]
patch

I'm fine with this, but if it's not needed after bug 554449 is fixed maybe we shouldn't do it?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/44090f6670c7

Well, I pushed the patch anyway. If security checks are changed to work
in a different way, this can be removed, and also nsCxPushers in ELM.

Comment 10

[Daniel Veditz [:dveditz]]

The testcase in this bug is now fixed by bug 554449. Do we want this patch on the branches anyway?