571 bytes, application/octet-stream
749 bytes, application/octet-stream
9.80 KB, patch
|Details | Diff | Splinter Review|
6.03 KB, patch
|Details | Diff | Splinter Review|
3.77 KB, patch
|Details | Diff | Splinter Review|
Alex Dvorov reported this to email@example.com: 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.
Bah, I thought we had mochitests for this when we fixed it last time :-(. bz, do you want this one?
Component: DOM: Core & HTML → Document Navigation
QA Contact: general → docshell
Whiteboard: [sg:moderate] → [sg:high]
> 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?
where is the testcase? Why do we get non-null principal if JS is not running o_O. XPConnect should push null cx.
> 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.
But click is happening in content, right? It should push the cx of content window.
..for event listeners. But for default handling there shouldn't be a context pushed, or, again, the context of the page.
Here is testcase: http://www.youtube.com/watch?v=HladcNKvlhQ
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.
Created attachment 573541 [details] [diff] [review] patch Boris, feel free to test this. (I'll test the patch once this meeting is over)
Hmm, no the patch doesn't affect to the behavior. And Chrome gives the same behavior.
So the patch does change principal handling. nsDocShell::ValidateOrigin gets null principal and not system principal. But the behavior is still the same.
So in my test case we're passing the same treeitem as origin and target :/
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.
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.
Ok, I shouldn't put <iframe src="http://ZZZZ/iframe.html"></iframe>, but open iframe.html in a separate tab.
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.
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.
Thanks for the tests, but any chance you could use zip or gz or bz2 for compression?
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.
(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.
Created attachment 573834 [details] [diff] [review] patch + tests Had to add a way to push null context.
In v8.1 this will be fixed?
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.
Okay. There are people who speak in Russian? Because it's very hard to write in English. Знает кто нибудь тут русский?
Я знаю русский.
О, это хорошо. А то я думал, что тут только по английски можно писать. То есть патч уже есть, осталось его только добавить в следующий FF 8.0.*?
Обычно, здесь лучше писать по английски, чтобы все понимали... Патч естъ. Его нужно проверить, потом добавитъ в 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).
I listen about Bug Bountry, can I get anything? :)
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).
I just need to wait 60 days, or must do something else?
(In reply to alexdvorov from comment #32) Just wait, and someone will be in contact with you eventually.
Okay. I'll wait.
Olli, can you explain the rationale here and what exactly is going wrong here?
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.
Also we can use top.location.href to do physhing :) It's true sg:high bug.
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>
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.
So, I can't give bouty or t-shirt?
Attachment #579768 - Attachment description: tests onlye → tests only
Created attachment 579771 [details] [diff] [review] patch only
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Comment on attachment 579771 [details] [diff] [review] patch only Per todays dirver meeting this is too late for beta, but approving for aurora.
status-firefox10: --- → fixed
status-firefox11: --- → fixed
Don't forget to check the tests in.
status1.9.2: --- → unaffected
Fails on Firefox 10.0.2. Should this be addressed in a ESR release?
status-firefox10: fixed → affected
Also fails on Firefox 11 Beta 6. Reopening this bug.
Status: RESOLVED → REOPENED
status-firefox11: fixed → affected
Resolution: FIXED → ---
Also broken in Aurora and Nightly.
Jason, how did you test?
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.
> 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.
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.
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.
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago → 7 years ago
status-firefox11: affected → fixed
Resolution: --- → FIXED
Verified fixed on FF 10, FF 11 Beta 6, FF 12 Aurora, FF 13 Nightly.
Status: RESOLVED → VERIFIED
status-firefox-esr10: --- → fixed
tracking-firefox-esr10: --- → 10+
(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?
You need to log in before you can comment on or make changes to this bug.