848253 - (CVE-2013-1709) It's possible to set a document's URI to a different document's URI

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[moz_bug_r_a4]

When a replace load happens in a subframe, nsDocShell::AddToSessionHistory reuses an existing SHEntry without creating a new BFCache entry. This can cause nsDocShell::InternalLoad to do a short-circuited load between different documents. By using this bug, it's possible to set a document's URI to a different document's URI.

* An attacker can create a fake login page whose URI is a real login page's URI. And, if a password for the real login page was already saved in the Password Manager, an attacker can steal the password without user interaction.

* An attacker can inject scripts into a page that uses external scripts with relative URLs.

Comment 1

[moz_bug_r_a4]

Attachment: testcase 1 - fake login page

This works on fx17-22.

Target page is https://www.getpersonas.com/en-US/signin

This creates a fake login page whose URI is the target page's URI. And, if you already saved a password for the target page, the Password Manager will automatically fill in the password.

Comment 2

[moz_bug_r_a4]

Attachment: testcase 2 - XSS

This works on fx17-22.

Target page is https://wiki.mozilla.org/Main_Page

This injects a script that opens an alert dialog with cookies into the target page.

Comment 3

[moz_bug_r_a4]

Please use the following URL to run testcase 2:

jar:https://bugzilla.mozilla.org/attachment.cgi?id=721604!/x.html

Comment 5

[Johnny Stenback (:jst)]

Olli, could you dig in here?

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Olli any luck?

Comment 7

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

Sorry about the delay. I'm going trough all my sg* bugs this week and this one is the last high/crit
rated so first one in the todo list.

Comment 8

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

(In reply to moz_bug_r_a4 from comment #1)
> Created attachment 721601 [details]
> testcase 1 - fake login page
> 
> This works on fx17-22.
> 
> Target page is https://www.getpersonas.com/en-US/signin
Looks like getpersonas page has changed.

Comment 9

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

(In reply to moz_bug_r_a4 from comment #2)
> Created attachment 721604 [details]
> testcase 2 - XSS
> 
> This works on fx17-22.
> 
> Target page is https://wiki.mozilla.org/Main_Page
> 
> This injects a script that opens an alert dialog with cookies into the
> target page.

I don't see any alert dialog when loading jar:https://bugzilla.mozilla.org/attachment.cgi?id=721604!/x.html
but the address of the iframe page looks wrong.

Comment 10

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

Attachment: v1

I think we should do this. I wish replace/bfcache handling wasn't so fragile.

Comment 11

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

Figuring out what kind of tests would be good for this. Tests should land way after the patch has landed though.

Comment 12

[moz_bug_r_a4]

(In reply to Olli Pettay [:smaug] from comment #8)
> (In reply to moz_bug_r_a4 from comment #1)
> > Created attachment 721601 [details]
> > testcase 1 - fake login page
> > 
> > This works on fx17-22.
> > 
> > Target page is https://www.getpersonas.com/en-US/signin
> Looks like getpersonas page has changed.

(In reply to Olli Pettay [:smaug] from comment #9)
> (In reply to moz_bug_r_a4 from comment #2)
> > Created attachment 721604 [details]
> > testcase 2 - XSS
> > 
> > This works on fx17-22.
> > 
> > Target page is https://wiki.mozilla.org/Main_Page
> > 
> > This injects a script that opens an alert dialog with cookies into the
> > target page.
> 
> I don't see any alert dialog when loading
> jar:https://bugzilla.mozilla.org/attachment.cgi?id=721604!/x.html
> but the address of the iframe page looks wrong.

testcase 1 and 2 no longer work because the target pages have changed.  Also, testcase 1 needs a trivial change to work on trunk.  I'll attach new testcases.

Comment 13

[moz_bug_r_a4]

Attachment: testcase 3 - fake login page

This works on fx17-23.

Target page is https://www.rooms.hp.com/signin.aspx

This creates a fake login page whose URI is the target page's URI. And, if you already saved a password for the target page, the Password Manager will automatically fill in the password.

Comment 14

[moz_bug_r_a4]

Attachment: testcase 4 - XSS

This works on fx17-23.

Target page is http://www8.hp.com/us/en/sitemap.html

This injects a script that opens an alert dialog with cookies into the target page.

Comment 15

[moz_bug_r_a4]

Please use the following URL to run testcase 4:

jar:https://bugzilla.mozilla.org/attachment.cgi?id=747847!/x.html

Comment 16

[Justin Lebar (not reading bugmail)]

smaug and I discussed this over PM.  Here's a slightly edited version.

<jlebar> I wanted to make sure I understood what's going on here.
<jlebar> We're doing a replace-history load into a subframe, but then the testcase causes us to end up using that shentry's bfcache entry after we replace it?
<jlebar> But the bfcache is the only thing that's wrong here -- e.g. the URI isn't wrong?
<smaug> so the key thing is the short-circuit load when going back
<jlebar> ah
<jlebar> as usual
<smaug> we end up calling SetDocumentURI and what not
<jlebar> okay, I see.

Comment 17

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

I'll try the new testcases.

Comment 18

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

Comment on attachment 747174 [details] [diff] [review]
v1

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not very easily, IMO

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Comment will be about releasing old data earlier

Which older supported branches are affected by this flaw?
All

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The same patch should apply everywhere.

How likely is this patch to cause regressions; how much testing does it need?
Changes to session history are somewhat regression risky.

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 747174 [details] [diff] [review]
v1

sec-approval+ for m-c after 5/14. Please nominate patches for branches then.

Comment 20

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/1695bdfbfa8e

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1695bdfbfa8e

Comment 22

[Alex Keybl [:akeybl]]

(In reply to Olli Pettay [:smaug] from comment #20)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/1695bdfbfa8e

Can you nominate for uplift to FF22/23 please?

Comment 23

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

Comment on attachment 747174 [details] [diff] [review]
v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): NA 
User impact if declined: see comment 0
Testing completed (on m-c, etc.): landed to m-c 2013-05-15
Risk to taking this patch (and alternatives if risky): 
Changes to session history are somewhat regression risky. I don't know any alternative approaches.
String or IDL/UUID changes made by this patch: NA

Comment 24

[Alex Keybl [:akeybl]]

We'll discuss whether we want to ship this first with FF22/23.

Comment 25

[Alex Keybl [:akeybl]]

Would also be great if you could provide instructions for general QA regression testing here given the risk profile.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/72f386b8c46b

Comment 27

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

(In reply to Alex Keybl [:akeybl] from comment #25)
> Would also be great if you could provide instructions for general QA
> regression testing here given the risk profile.

See moz_bug_r_a4's tests

Comment 28

[u279076]

Matt, can you please have a look at this RE: comment 27?

Comment 29

[Matt Wobensmith [:mwobensmith][:matt:]]

This is fairly straightforward to verify, but I can't get testcase 3 (in comment 15) to work, as Firefox won't allow me to open a file with jar: protocol. Am I missing something?

The other test (testcase 3) works fine.

Comment 30

[Matt Wobensmith [:mwobensmith][:matt:]]

^^^ Replace first line with this:

"This is fairly straightforward to verify, but I can't get testcase 4 (in comment 15) to work (...)"

Comment 31

[:Gavin Sharp [email: [e-mail]]]

You'll need to flip network.jar.open-unsafe-types to true for that particular testcase to work, I think.

Comment 32

[:Gavin Sharp [email: [e-mail]]]

Hmm, scratch that, it's marked as "application/java-archive", so it should work either way.

Comment 33

[Matt Wobensmith [:mwobensmith][:matt:]]

Thanks for the help, Gavin. Flipping the pref worked.

For the record, I confirmed the vulnerability based on the following assumptions:

- Testcase 3 is able to mimic a page in hp.com's domain, which prompts the Password Manager for that domain and displays hp.com's domain in the address bar.

- Testcase 4 is able to inject a JS alert box, which tries to display the hp.com domain's cookie.

I confirmed a successful fix for these vulns with these assumptions:

- Testcase 3 no longer prompts the Password Manager for hp.com's domain, and no longer displays their domain in the address bar, but "data:," instead.

- Testcase 4 no longer displays a JS alert box at all.

I was able to see both test cases work in pre-patch builds from multiple branches.

I was able to verify fixed on these branches:

FF23 2013-05-22
FF24 2013-05-22

Comment 34

[Alex Keybl [:akeybl]]

Comment on attachment 747174 [details] [diff] [review]
v1

Sorry, mismarked for a moment there. We decided to take in FF23 and all corresponding security branches (including B2G v1.1).

Comment 35

[Alex Keybl [:akeybl]]

(the rationale was of course the risk described in comment 23, and an email discussion with the security team)

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/0c187b6751a2
https://hg.mozilla.org/releases/mozilla-esr17/rev/3aff426f2461

Comment 37

[Matt Wobensmith [:mwobensmith][:matt:]]

Confirmed fixed 17esr 2013-06-18.