701071 - (CVE-2012-0445) <iframe> element is exposed across domains by its name attribute

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Brandon Sterne (:bsterne)]

Alex Dvorov reported this to security@m.o:

A form can be submitted to a named iframe in another domain if you know the name attribute of the iframe element.

This is a regression, as Firefox 3.6 creates a new top-level window to which the form is submitted if the named iframe doesn't exist within the current window.  This is the same behavior as the current version of Chrome.

I'm setting this as moderate severity, because most high bugs involve stealing information or running script in the context of another site, but this is still bad. You could easily use this to spoof the content in another site if you know the name of an iframe there.  I could be persuaded that this is in fact a high bug.

Comment 1

[Benjamin Smedberg]

Bah, I thought we had mochitests for this when we fixed it last time :-(. bz, do you want this one?

Comment 2

[Boris Zbarsky [:bzbarsky]]

> I thought we had mochitests for this when we fixed it last time

Hmm.  I believe at the time there was no way to sanely write tests for it, but I'll check.

In any case, what's happening is that the GetSubjectPrincipal call in nsDocShell::ValidateOrigin is finding the system principal, not null as it should for the form submit case.  And it treats the two differently.

And the reason GetSubjectPrincipal does that is that nsScriptSecurityManager::GetPrincipalAndFrame extracts an nsJSContext from the |cx| which has an nsGlobalChromeWindow as its GetObjectPrincipal.

The callstack at this point has no JS running on it, but does have the click event dispatch that triggers the form submission...

Is a non-null subject principal actually expected in this situation?  If so, why?

Comment 3

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

where is the testcase?

Why do we get non-null principal if JS is not running o_O. XPConnect should push
null cx.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> where is the testcase?

The testcase I used looks like this:

On server ZZZZ I put this file, named iframe.html:

  <iframe name="x"></iframe>

On server B (file:// in my case), I put this file:

  <form action="http://www.mozilla.org" target="x">
    <input type="submit">
  </form>
  <iframe src="http://ZZZZ/iframe.html"></iframe>

Then clicked the submit button.

> Why do we get non-null principal if JS is not running

I _think_ that dispatch of the click event pushes the cx of the chrome window or something.  I didn't get a chance to debug more in detail yet.

Comment 5

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

But click is happening in content, right? It should push the cx of content window.

Comment 6

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

..for event listeners. But for default handling there shouldn't be a context pushed, or, again,
the context of the page.

Comment 7

[alexdvorov]

Here is testcase: http://www.youtube.com/watch?v=HladcNKvlhQ

Comment 8

[Boris Zbarsky [:bzbarsky]]

That's not a testcase; that's a video.  A testcase would be a web page showing the problem.

Olli, per our IRC conversation, over to you.  Looks like the issue is that we RePush when calling nsEventListenerManager::HandleEventInternal on the tabbrowser and pop too late.

Comment 9

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

Attachment: patch

Boris, feel free to test this.

(I'll test the patch once this meeting is over)

Comment 10

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

Hmm, no the patch doesn't affect to the behavior.
And Chrome gives the same behavior.

Comment 11

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

So the patch does change principal handling. nsDocShell::ValidateOrigin gets
null principal and not system principal. But the behavior is still the same.

Comment 12

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

So in my test case we're passing the same treeitem as origin and target :/

Comment 13

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

alexdvorov from comment #7)
> Here is testcase: http://www.youtube.com/watch?v=HladcNKvlhQ

Would be great if you could upload your test using "Add an attachment", since apparently
I have a different kind of testcase (the one from Boris) which  is actually broken also in FF3.6.

Comment 14

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

I clearly have wrong test case (though, it is still showing a problem in browsers) since it
works the same way in Gecko, Webkit and IE.

Comment 15

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

Ok, I shouldn't put <iframe src="http://ZZZZ/iframe.html"></iframe>, but open
iframe.html in a separate tab.

Comment 16

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

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

Make sure we pop cx from stack when doing default handling.
This is a regression from year 2009.

I need to still write the mochitest for this.

Comment 17

[alexdvorov]

I from Russia, I can do mistakes in my messages, and incorrectly understand words. 
I uploaded tests. It's not nesessary to run pages from different domains.

Comment 18

[alexdvorov]

Attachment: tests

Comment 19

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

Thanks for the tests, but any chance you could use zip or gz or bz2 for compression?

Comment 20

[alexdvorov]

Attachment: Zip comressed tests

Comment 21

[alexdvorov]

Added zip comressed testcase. Just open in two tabs index.html and index2.html. 
And why platform is Linux only? Under Windows it works too.

Comment 22

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

(In reply to alexdvorov from comment #17)
>  It's not nesessary to run pages from different domains.
Well, that is then a different case. The regression I see need cross-domain.

Comment 23

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

Attachment: patch + tests

Had to add a way to push null context.

Comment 24

[alexdvorov]

In v8.1 this will be fixed?

Comment 25

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

There won't be 8.1, and I doubt this can make to 8.0.*, since this isn't *critical* security bug.
But I hope FF9, at latest. That will be released before Christmas.

Anyhow, the patch needs to be reviewed before it can land to any branch.

Comment 26

[alexdvorov]

Okay. There are people who speak in Russian? Because it's very hard to write in English. 
Знает кто нибудь тут русский?

Comment 27

[Boris Zbarsky [:bzbarsky]]

Я знаю русский.

Comment 28

[alexdvorov]

О, это хорошо. А то я думал, что тут только по английски можно писать.
То есть патч уже есть, осталось его только добавить в следующий FF 8.0.*?

Comment 29

[Boris Zbarsky [:bzbarsky]]

Обычно, здесь лучше писать по английски, чтобы все понимали...

Патч естъ.  Его нужно проверить, потом добавитъ в FF 11, потом скорее всего в 10 и 9.

В 8.0.* этот патч скорее всего не попадет потому, что это не критический баг (пример критического бага -- возможность запускать на компьютере пользователя произвольный код).

-------

For the rest, what happened above is Alex said that he's glad someone understands Russian; he'd thought that he would have to stick to English here.  He then asked whether he understands correctly that there is a patch and all that's left is to add it to the next FF 8.0.*.

What I said is basically a repeat of comment 25, but in Russian, along with an example of what we consider a critical security bug (remote code execution).

Comment 30

[alexdvorov]

I listen about Bug Bountry, can I get anything? :)

Comment 31

[Lucas Adamski [:ladamski]]

New sg:high and sg:critical bugs are generally considered eligible for the bounty, but a formal decision will be made when the bounties evaluated (usually every 60 days or so).

Comment 32

[alexdvorov]

I just need to wait 60 days, or must do something else?

Comment 33

[Brandon Sterne (:bsterne)]

(In reply to alexdvorov from comment #32)

Just wait, and someone will be in contact with you eventually.

Comment 34

[alexdvorov]

Okay. I'll wait.

Comment 35

[Johnny Stenback (:jst)]

Olli, can you explain the rationale here and what exactly is going wrong here?

Comment 36

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

We end up having wrong cx (chrome) on js stack when doing PostHandleEvent (which ends up submitting 
the form).
This is a regression from the work where the number of push/pop calls  were tried to be reduced.

Comment 37

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

review ping

Comment 38

[alexdvorov]

Also we can use top.location.href to do physhing :)
It's true sg:high bug.

Comment 39

[alexdvorov]

Sorry - fishing.
We can change all page containing iframe element, if page that was be in iframe, contains 
<script>top.location.href = "http://google.com/";</script>

Comment 41

[Daniel Veditz [:dveditz]]

The frame targeting issue has been considered sg:moderate in the past (e.g. http://www.mozilla.org/security/announce/2005/mfsa2005-51.html ) but it's possible we've learned better how to exploit it.

But having the wrong cx on the stack (comment 36) sounds like it might lead to much worse (or not?). CC'ing moz_bug_r_a4 in case this inspires him.

Comment 42

[alexdvorov]

So, I can't give bouty or  t-shirt?

Comment 43

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

Attachment: tests only

I'll land the patch first.

Comment 44

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

Attachment: patch only

Comment 45

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

https://hg.mozilla.org/mozilla-central/rev/dfff3e59ef23

Comment 46

[Johnny Stenback (:jst)]

Comment on attachment 579771 [details] [diff] [review]
patch only

Per todays dirver meeting this is too late for beta, but approving for aurora.

Comment 47

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/9871079caf78

Comment 48

[Daniel Veditz [:dveditz]]

Don't forget to check the tests in.

Comment 49

[Jason Smith [:jsmith]]

Fails on Firefox 10.0.2. Should this be addressed in a ESR release?

Comment 50

[Jason Smith [:jsmith]]

Also fails on Firefox 11 Beta 6. Reopening this bug.

Comment 51

[Jason Smith [:jsmith]]

Also broken in Aurora and Nightly.

Comment 52

[Boris Zbarsky [:bzbarsky]]

Jason, how did you test?

Comment 53

[Jason Smith [:jsmith]]

Steps:

1. Download the https://bugzilla.mozilla.org/attachment.cgi?id=573782
2. Opened index.html in tab #1
3. Opened index2.html in tab #2
4. Select the POST button in index2.html

Expected:

The text in index.html should not change.

Actual:

The text in index.html did change.

Comment 54

[Boris Zbarsky [:bzbarsky]]

> 2. Opened index.html in tab #1
> 3. Opened index2.html in tab #2

On different servers?  The bug report is about the case when index.html is on one server and index2.html is on another one.  See comment 4.

Comment 55

[Jason Smith [:jsmith]]

My test cases were both executed on the same server. I can retest that with separate servers. 

However, I want to know if that test case I used is allowed where index.html and index2.html are hosted on the same server. If it isn't, then there's still a problem.

Comment 56

[Boris Zbarsky [:bzbarsky]]

If they're on the same server, they should be able to link to each other at the moment.

Resetting the fixed state, since as far as I can tell the bug is in fact fixed.

Comment 57

[Jason Smith [:jsmith]]

Verified fixed on FF 10, FF 11 Beta 6, FF 12 Aurora, FF 13 Nightly.

Comment 58

[u279076]

(In reply to Jason Smith [:jsmith] from comment #57)
> Verified fixed on FF 10, FF 11 Beta 6, FF 12 Aurora, FF 13 Nightly.

Jason, are you still set up to test this? If so, can you test the latest ESR nightly?

Comment 59

[Raymond Forbes[:rforbes]]

rforbes-bugspam-for-setting-that-bounty-flag-20130719