Last Comment Bug 701071 - (CVE-2012-0445) <iframe> element is exposed across domains by its name attribute
(CVE-2012-0445)
: <iframe> element is exposed across domains by its name attribute
Status: VERIFIED FIXED
[sg:high][qa?]
: regression
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-09 09:59 PST by Brandon Sterne (:bsterne)
Modified: 2015-10-07 18:43 PDT (History)
15 users (show)
rforbes: sec‑bounty+
dveditz: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified
verified
10+
fixed
unaffected


Attachments
patch (3.77 KB, patch)
2011-11-10 09:31 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
tests (571 bytes, application/octet-stream)
2011-11-10 14:15 PST, alexdvorov
no flags Details
Zip comressed tests (749 bytes, application/octet-stream)
2011-11-11 06:04 PST, alexdvorov
no flags Details
patch + tests (9.80 KB, patch)
2011-11-11 09:36 PST, Olli Pettay [:smaug]
jst: review+
Details | Diff | Splinter Review
tests only (6.03 KB, patch)
2011-12-07 11:30 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
patch only (3.77 KB, patch)
2011-12-07 11:43 PST, Olli Pettay [:smaug]
jst: approval‑mozilla‑aurora+
jst: approval‑mozilla‑beta-
Details | Diff | Splinter Review

Description Brandon Sterne (:bsterne) 2011-11-09 09:59:55 PST
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 [:bsmedberg] 2011-11-09 10:17:56 PST
Bah, I thought we had mochitests for this when we fixed it last time :-(. bz, do you want this one?
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 22:41:25 PST
> 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] 2011-11-10 02:25:50 PST
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 [:bz] (still a bit busy) 2011-11-10 07:00:42 PST
> 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] 2011-11-10 07:04:03 PST
But click is happening in content, right? It should push the cx of content window.
Comment 6 Olli Pettay [:smaug] 2011-11-10 07:05:47 PST
..for event listeners. But for default handling there shouldn't be a context pushed, or, again,
the context of the page.
Comment 7 alexdvorov 2011-11-10 07:56:32 PST
Here is testcase: http://www.youtube.com/watch?v=HladcNKvlhQ
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2011-11-10 08:55:43 PST
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] 2011-11-10 09:31:03 PST
Created attachment 573541 [details] [diff] [review]
patch

Boris, feel free to test this.

(I'll test the patch once this meeting is over)
Comment 10 Olli Pettay [:smaug] 2011-11-10 09:54:26 PST
Hmm, no the patch doesn't affect to the behavior.
And Chrome gives the same behavior.
Comment 11 Olli Pettay [:smaug] 2011-11-10 10:17:22 PST
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] 2011-11-10 10:53:36 PST
So in my test case we're passing the same treeitem as origin and target :/
Comment 13 Olli Pettay [:smaug] 2011-11-10 12:19:05 PST
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] 2011-11-10 12:47:38 PST
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] 2011-11-10 13:02:04 PST
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] 2011-11-10 13:08:08 PST
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 2011-11-10 14:14:24 PST
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 2011-11-10 14:15:10 PST
Created attachment 573638 [details]
tests
Comment 19 Olli Pettay [:smaug] 2011-11-10 14:33:39 PST
Thanks for the tests, but any chance you could use zip or gz or bz2 for compression?
Comment 20 alexdvorov 2011-11-11 06:04:56 PST
Created attachment 573782 [details]
Zip comressed tests
Comment 21 alexdvorov 2011-11-11 06:08:48 PST
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] 2011-11-11 09:34:45 PST
(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] 2011-11-11 09:36:59 PST
Created attachment 573834 [details] [diff] [review]
patch + tests

Had to add a way to push null context.
Comment 24 alexdvorov 2011-11-11 12:17:25 PST
In v8.1 this will be fixed?
Comment 25 Olli Pettay [:smaug] 2011-11-11 12:20:39 PST
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 2011-11-11 12:29:29 PST
Okay. There are people who speak in Russian? Because it's very hard to write in English. 
Знает кто нибудь тут русский?
Comment 27 Boris Zbarsky [:bz] (still a bit busy) 2011-11-12 00:26:25 PST
Я знаю русский.
Comment 28 alexdvorov 2011-11-12 03:09:45 PST
О, это хорошо. А то я думал, что тут только по английски можно писать.
То есть патч уже есть, осталось его только добавить в следующий FF 8.0.*?
Comment 29 Boris Zbarsky [:bz] (still a bit busy) 2011-11-12 10:10:22 PST
Обычно, здесь лучше писать по английски, чтобы все понимали...

Патч естъ.  Его нужно проверить, потом добавитъ в 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 2011-11-13 10:04:43 PST
I listen about Bug Bountry, can I get anything? :)
Comment 31 Lucas Adamski [:ladamski] 2011-11-14 10:56:11 PST
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 2011-11-14 12:54:26 PST
I just need to wait 60 days, or must do something else?
Comment 33 Brandon Sterne (:bsterne) 2011-11-14 13:17:06 PST
(In reply to alexdvorov from comment #32)

Just wait, and someone will be in contact with you eventually.
Comment 34 alexdvorov 2011-11-14 13:27:44 PST
Okay. I'll wait.
Comment 35 Johnny Stenback (:jst, jst@mozilla.com) 2011-11-15 17:56:17 PST
Olli, can you explain the rationale here and what exactly is going wrong here?
Comment 36 Olli Pettay [:smaug] 2011-11-16 00:52:41 PST
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] 2011-11-18 09:22:16 PST
review ping
Comment 38 alexdvorov 2011-11-19 21:38:01 PST
Also we can use top.location.href to do physhing :)
It's true sg:high bug.
Comment 39 alexdvorov 2011-11-20 03:22:32 PST
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] 2011-11-29 13:24:55 PST
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 2011-12-03 14:01:30 PST
So, I can't give bouty or  t-shirt?
Comment 43 Olli Pettay [:smaug] 2011-12-07 11:30:44 PST
Created attachment 579768 [details] [diff] [review]
tests only

I'll land the patch first.
Comment 44 Olli Pettay [:smaug] 2011-12-07 11:43:43 PST
Created attachment 579771 [details] [diff] [review]
patch only
Comment 45 Olli Pettay [:smaug] 2011-12-07 11:44:53 PST
https://hg.mozilla.org/mozilla-central/rev/dfff3e59ef23
Comment 46 Johnny Stenback (:jst, jst@mozilla.com) 2011-12-08 14:49:11 PST
Comment on attachment 579771 [details] [diff] [review]
patch only

Per todays dirver meeting this is too late for beta, but approving for aurora.
Comment 48 Daniel Veditz [:dveditz] 2012-01-12 18:36:05 PST
Don't forget to check the tests in.
Comment 49 Jason Smith [:jsmith] 2012-03-06 17:52:45 PST
Fails on Firefox 10.0.2. Should this be addressed in a ESR release?
Comment 50 Jason Smith [:jsmith] 2012-03-06 17:55:27 PST
Also fails on Firefox 11 Beta 6. Reopening this bug.
Comment 51 Jason Smith [:jsmith] 2012-03-06 17:57:31 PST
Also broken in Aurora and Nightly.
Comment 52 Boris Zbarsky [:bz] (still a bit busy) 2012-03-06 18:23:05 PST
Jason, how did you test?
Comment 53 Jason Smith [:jsmith] 2012-03-06 18:25:07 PST
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 [:bz] (still a bit busy) 2012-03-06 18:33:34 PST
> 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] 2012-03-06 18:40:09 PST
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 [:bz] (still a bit busy) 2012-03-06 19:13:52 PST
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] 2012-03-06 20:21:33 PST
Verified fixed on FF 10, FF 11 Beta 6, FF 12 Aurora, FF 13 Nightly.
Comment 58 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-21 17:05:09 PDT
(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] 2013-07-19 18:27:15 PDT
rforbes-bugspam-for-setting-that-bounty-flag-20130719

Note You need to log in before you can comment on or make changes to this bug.