1730120 - (CVE-2021-43536) UXSS: location.origin is changed (port/host)

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Sunwoo Kim]

Firefox version: firefox-91.0b9
OS: Ubuntu 18.04.1

There is a console.log printing location.origin on poc.html #9(console.log(location.origin);).
If you turn on the devtools.console.stdout.content setting and run firefox, you can see that location.origin is changed.

The part to change the port or host of origin is line 34 and 35 in poc.html.

v96.port = 1234;

It seems that changing the host does not work well, but it is possible to change it to something else arbitrarily.

• reproduce
  host poc.html at http://127.0.0.1:7000/poc.html then

~/firefox/firefox-91.0b9/firefox --headless http://127.0.0.1:7000/poc.html 

result is here.

*** You are running in headless mode.
[GFX1-]: glxtest: Unable to open a connection to the X server
[GFX1-]: glxtest: libEGL initialize failed
[GFX1-]: RenderCompositorSWGL failed mapping default framebuffer, no dt
console.log: "http://127.0.0.1:7000"
console.log: "http://127.0.0.1:7000"
console.log: "http://127.0.0.1:7000"
console.log: "http://127.0.0.1:7000"
console.log: "http://127.0.0.1:7000"
console.log: "http://127.0.0.1:1234"
console.log: "http://127.0.0.1:1234"
console.log: "http://127.0.0.1:1234"
console.log: "http://127.0.0.1:1234"
console.log: "http://127.0.0.1:1234"
console.log: "http://127.0.0.1:1234"
console.log: "http://127.0.0.1:1234"

• rr log (rr_log.txt)

rr record ~/firefox/firefox-91.0b9/firefox --headless http://127.0.0.1:7000/poc.html
rr -M replay -a | tee rr_log.txt

[rr 27391 1683]*** You are running in headless mode.
[rr 27391 4310][GFX1-]: glxtest: Unable to open a connection to the X server
[rr 27391 4314][GFX1-]: glxtest: libEGL initialize failed
[rr 27391 17889][GFX1-]: RenderCompositorSWGL failed mapping default framebuffer, no dt
[rr 27524 26611]console.log: "http://127.0.0.1:7000"
[rr 27524 28826]console.log: "http://127.0.0.1:7000"
[rr 27524 30135]console.log: "http://127.0.0.1:7000"
[rr 27524 32311]console.log: "http://127.0.0.1:7000"
[rr 27524 32989]console.log: "http://127.0.0.1:7000"
[rr 27524 35930]console.log: "http://127.0.0.1:1234"
[rr 27524 36783]console.log: "http://127.0.0.1:1234"
[rr 27524 37772]console.log: "http://127.0.0.1:1234"
[rr 27524 38782]console.log: "http://127.0.0.1:1234"
[rr 27524 39478]console.log: "http://127.0.0.1:1234"
[rr 27524 40252]console.log: "http://127.0.0.1:1234"
[rr 27524 40887]console.log: "http://127.0.0.1:1234"

Comment 1

[Sunwoo Kim]

rr recording result is too big to upload here.
I uploaded it to my server

$ wget http://147.46.125.79:7000/firefox-0.tar.gz

Comment 2

[Sunwoo Kim]

Attachment: report.tar.gz

Comment 3

[:Gijs (he/him)]

Unsure why this report hasn't been touched for a while; Tentatively moving to DOM. Nika, can you (find someone to) take a look at what's happening here? It looks potentially bad...

Comment 4

[Andrew McCreight [:mccr8]]

Nika looked over this in a meeting we had today. From what she said, it sounds like the poc.html is being hosted at both port 7000 and port 1234. Setting the port causes us to navigate from http://127.0.0.1:7000/poc.html to http://127.0.0.1:1234/poc.html, which then starts logging on the console with the new URL, but there's no SOP violation because we're now running the new page. Are you seeing something different than this? What does the URL bar show when we're in the "console.log: "http://127.0.0.1:1234"" state?

Comment 5

[Daniel Veditz [:dveditz]]

This one appears to be invalid -- you've navigated to the same content on a different port, and it reports that correctly. (different from bug 1727480 which does have different content on the two ports)

Comment 6

[Sunwoo Kim]

Attachment: record.mp4

reproduce demo

Comment 7

[Sunwoo Kim]

Attachment: not_bug.mp4

navigated case

Comment 8

[Sunwoo Kim]

Attachment: capture.png

Comment 9

[Sunwoo Kim]

Attachment: code.tar.gz

Sorry for late response. Please check this again.
I think this might not be UXSS bug, but still think this is a bug.

First, I check the your comment(same content on different port) and it is not correct.
To show this, I uploaded test code.

There are 2 servers and 2 html files. Each html files are hosted on each servers
poc.html is hosted on http://127.0.0.1:5001
sample.html is hosted on http://127.0.0.1:5002

In order to distinguish the two, sample text is written in sample.html.

If you look at the uploaded record.mp4 (I don't know why, but I have to download it because it can't be played on the web.),
you can see console.log print http://127.0.0.1:5002, after navigated to http://127.0.0.1:5002.
And URL bar had http://127.0.0.1:5001 (I add sleep to see more long time)
In my opinion, navigation is not actually done.

Now look at the not_bug.mp4, it is actually navigated to http://127.0.0.1:5002.
This happens when you already have many tabs open.

I cannot make I haven't come up with a bug that violates the SOP yet, but this behavior certainly looks odd.
If there's anything I'm doing wrong, please let me know.

Thank you

Comment 10

[Daniel Veditz [:dveditz]]

reopening to look at the new testcase/movie

Comment 11

[Andrew McCreight [:mccr8]]

Nika figured out some interesting things going on here in the test case. Olli said he'd take a look and write up some of what we figured out, because I did not entirely understand everything. It sounded kind of like a navigation issue, so I'll move it there for now.

Comment 12

[Sunwoo Kim]

Could you please share your progress?

Comment 13

[Sunwoo Kim]

Attachment: fetch_result.png

Comment 14

[Sunwoo Kim]

Attachment: SOP bypass poc

I found the SOP bypass PoC.
This is a slightly modified code of the previously uploaded one.

• poc.html
  change function to async and add fetch url

  async function f2 () {
    if (location.origin != "http://127.0.0.1:5001"){
          if (flag == 0){
            console.log(location.origin)
            console.log(await (await fetch("http://127.0.0.1:5002/danger", {method: 'POST'})).text());
            sleep(10000)
            flag = 1
          }
      }
      else{
        try {v60 = v14.open();} catch(e) {} 
      }
  }

• app_child.py
  add new api entry

@app.route('/danger', methods=['POST'])
def danger():
    print(request.headers)
    return 'Hello, Child!'

you can check POST request on fetch_result.png.
If you look at the screenshot, you can see that app_child.py(which is hosted at http://127.0.0.1:5002) receive the request from origin http://127.0.0.1:5001.
And you can also check the request result on dev console.

Comment 15

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

(I'll try to get to this asap. Have been busy with other things)

Comment 16

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

https://searchfox.org/mozilla-central/rev/7588d077255c5732f72c466e71d7a38d97385dd5/dom/base/nsGlobalWindowOuter.cpp#2114 returns early so the docshell leaves in a bit odd state.
A patch coming

Comment 17

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

Attachment: Bug 1730120, close ContentViewer properly if initialization fails, r=nika

Comment 18

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

Comment on attachment 9248941 [details]
Bug 1730120, close ContentViewer properly if initialization fails, r=nika

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Probably not very easily
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: the relevant code should be the same in esr too
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, the issue happens only in a very rare edge case

Comment 19

[Andrew McCreight [:mccr8]]

Have you figured out what the security implications of this are? We need to rate this before it can land.

Comment 20

[Andrew McCreight [:mccr8]]

Olli said the issue is that we may potentially expose redirects cross origin, so I guess that's maybe sec-high?

Comment 21

[Tom Ritter [:tjr]]

Comment on attachment 9248941 [details]
Bug 1730120, close ContentViewer properly if initialization fails, r=nika

Approved to land and request uplift

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed: https://hg.mozilla.org/integration/autoland/rev/d2e38d5047ae4be0ce86ba1758b65177a8674a24

Backed out for causing Crashtest failures with crashtests/1419902.html:
https://hg.mozilla.org/integration/autoland/rev/7daf837d099295087648e3ca1fd8e7401eadfe7e

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=aZ8q8-tpQXKrgen_z-wtRQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=d2e38d5047ae4be0ce86ba1758b65177a8674a24
Failure log: https://treeherder.mozilla.org/logviewer?job_id=357538752&repo=autoland

dom/base/crashtests/1419902.html | assertion count 22 is more than expected 0 assertions

###!!! ASSERTION: No document in Destroy()!: 'mDocument', file /builds/worker/checkouts/gecko/layout/base/nsDocumentViewer.cpp:1600
#01: NS_DebugBreak [xpcom/base/nsDebugImpl.cpp:429]
#02: nsDocumentViewer::Destroy() [layout/base/nsDocumentViewer.cpp:1606]
#03: nsDocShell::SetupNewViewer(nsIContentViewer*, mozilla::dom::WindowGlobalChild*) [docshell/base/nsDocShell.cpp:8113]
#04: nsDocShell::Embed(nsIContentViewer*, mozilla::dom::WindowGlobalChild*, bool, bool, nsIRequest*) [docshell/base/nsDocShell.cpp:5586]
#05: nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIPrincipal*, nsIContentSecurityPolicy*, nsIURI*, bool, mozilla::Maybe<nsILoadInfo::CrossOriginEmbedderPolicy> const&, bool, bool, mozilla::dom::WindowGlobalChild*) [docshell/base/nsDocShell.cpp:6665]
#06: nsDocShell::EnsureContentViewer() [docshell/base/nsDocShell.cpp:6478]
#07: nsDocShell::GetInheritedPrincipal(bool, bool) [docshell/base/nsDocShell.cpp:9630]
#08: nsDocShell::CheckDisallowedJavascriptLoad(nsDocShellLoadState*) [docshell/base/nsDocShell.cpp:697]
#09: nsDocShell::LoadURI(nsDocShellLoadState*, bool, bool) [docshell/base/nsDocShell.cpp:730]
#10: nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy*, nsTSubstring<char> const&, nsTSubstring<char> const&, nsTSubstring<char> const&, bool, bool, bool, nsIArray*, bool, bool, bool, nsPIWindowWatcher::PrintKind, nsDocShellLoadState*, mozilla::dom::BrowsingContext**) [toolkit/components/windowwatcher/nsWindowWatcher.cpp:1273]
#11: nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy*, nsTSubstring<char> const&, nsTSubstring<char> const&, nsTSubstring<char> const&, bool, bool, bool, nsISupports*, bool, bool, bool, nsPIWindowWatcher::PrintKind, nsDocShellLoadState*, mozilla::dom::BrowsingContext**) [toolkit/components/windowwatcher/nsWindowWatcher.cpp:376]
#12: nsGlobalWindowOuter::OpenInternal(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, bool, bool, bool, bool, bool, nsIArray*, nsISupports*, nsDocShellLoadState*, bool, nsGlobalWindowOuter::PrintKind, mozilla::dom::BrowsingContext**) [dom/base/nsGlobalWindowOuter.cpp:7158]
#13: nsGlobalWindowOuter::OpenOuter(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) [dom/base/nsGlobalWindowOuter.cpp:5756]
#14: nsGlobalWindowInner::Open(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) [dom/base/nsGlobalWindowInner.cpp:3995]
#15: mozilla::dom::Window_Binding::open(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) [s3:gecko-generated-sources:5556839183182808c48ee2da456abfe78f940fd0d7ac582e834c351aa3199517e738391db6479266df08d82911d1da7ec6a97dfb0fc92fef12340c46bf819e98/dom/bindings/WindowBinding.cpp::2706]
#16: bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::MaybeCrossOriginObjectThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) [dom/bindings/BindingUtils.cpp:3302]

Comment 23

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

aha, I need to tweak the assertion.

Comment 24

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

The relevant method has even null checks for mDocument, yet there is that NS_ASSERTION for mDocument

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

close ContentViewer properly if initialization fails, r=nika
https://hg.mozilla.org/integration/autoland/rev/047151a9e454f43440e1484691c2bcb3a3d54c25
https://hg.mozilla.org/mozilla-central/rev/047151a9e454

Comment 26

[Sunwoo Kim]

Could I be assigned a CVE for this issue?
Since Mozilla is member of CNA, I thought CVE assignment would be possible.
If you don't manage CVE, can I apply this to MITRE?

Comment 27

[Andrew McCreight [:mccr8]]

(In reply to Sunwoo Kim from comment #26)

Could I be assigned a CVE for this issue?

Yes, a CVE will be assigned to it as part of our standard processes as we get closer to the first release that contains the fix. This bug will be updated with the CVE after it is assigned.

Comment 28

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 29

[Sunwoo Kim]

(In reply to Andrew McCreight [:mccr8] from comment #27)

Yes, a CVE will be assigned to it as part of our standard processes as we get closer to the first release that contains the fix. This bug will be updated with the CVE after it is assigned.

Please credit to Sunwoo Kim and Youngmin Kim of SNU CompSec Lab

Comment 30

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:smaug, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 31

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

Comment on attachment 9248941 [details]
Bug 1730120, close ContentViewer properly if initialization fails, r=nika

Beta/Release Uplift Approval Request

• User impact if declined: Exposing the url of the to-be-loaded page to the previous page
  (but the next page doesn't actually load)
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: run the test in the bug
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch changes code path which is taken in a very extreme edge case.
• String changes made/needed: NA

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 32

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Hey Olli, I was trying to reproduce the issue with the poc.html and two separate folders containing it while linking each folder to 127.0.0.1 address, one with 5001 port and the other one with 5002 port. I may be doing this wrong (I'll attach a pic with what I did, also a :1234 port can be seen in the console, and not sure why that was called). If this is not the way to reproduce the issue, could you please provide the exact steps in order to reproduce this issue? Thank you.

Comment 33

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Attachment: try repro.png

Here is the attachment, sorry for the delay.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9248941 [details]
Bug 1730120, close ContentViewer properly if initialization fails, r=nika

Approved for 95.0b9.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/be06b7c89c28

Comment 36

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9248941 [details]
Bug 1730120, close ContentViewer properly if initialization fails, r=nika

Approved for 91.4esr.

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/d350a75e6517

Comment 38

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Sunwoo, could you please eventually verify the fix for Firefox 91 ESR, Beta 95 and Nightly 96 if you have some time? Thank you!

Comment 39

[Sunwoo Kim]

Thank you for the link, I could easily test it.

It looks like everything ok :)

Comment 40

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Setting this issue as verified based on Comment 39. Thank you Sunwoo for looking over it!

Comment 41

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Hey Olli,

I've tried to write an advisory by looking at the bug report and the patch but I'm admittedly taking very wild guesses - even when making things vague.

Can you do me the favor and review the advisory text (or suggest some alternative)? (An example advisory and what kind of style we want to maintain is explained at https://wiki.mozilla.org/Security/Firefox/Security_Bug_Life_Cycle/Security_Advisories#Write_the_advisories).

Thanks!

Comment 42

[Frederik Braun [:freddy]]

We will be crediting you in the security advisories for our upcoming release of Firefox 95 as "smolikraphael".
Please respond to this needinfo request, if you wish to be credited differently (or not at all).

Comment 43

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

Comment on attachment 9253433 [details]
advisory.txt

>Redirect URL leakage when redirecting while executing asynchronous function

do we need to talk about redirect here. It is really leaking an url.


>Sunwoo Kim and Youngmin Kim of SNU CompSec Lab
>
>Under certain circumstances, asynchronous functions could cause a redirect to fail but expose the redirect target location.

It is not redirect which fails, but load.

Comment 44

[Andrew McCreight [:mccr8]]

(In reply to Frederik Braun [:freddy] from comment #42)

We will be crediting you in the security advisories for our upcoming release of Firefox 95 as "smolikraphael".
Please respond to this needinfo request, if you wish to be credited differently (or not at all).

I'm assuming Freddy meant to needinfo the reporter here.

Comment 45

[Frederik Braun [:freddy]]

(In reply to Andrew McCreight [:mccr8] from comment #44)

(In reply to Frederik Braun [:freddy] from comment #42)

We will be crediting you in the security advisories for our upcoming release of Firefox 95 as "smolikraphael".
Please respond to this needinfo request, if you wish to be credited differently (or not at all).

I'm assuming Freddy meant to needinfo the reporter here.

Awww no, that was meant for a different bug, somehow ended up as a the draft comment and was submitted when I set the CVE.
This one will be credited as "Sunwoo Kim and Youngmin Kim of SNU CompSec Lab"!

Sorry for the confusion!

Comment 46

[Frederik Braun [:freddy]]

Attachment: advisory.txt

thanks!