Open Bug 1424509 Opened 3 years ago Updated 3 years ago

Forcing browser to close using data: top navigation blocker

Categories

(Core :: DOM: Security, defect, P3)

58 Branch
defect

Tracking

()

People

(Reporter: qab, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-dos, Whiteboard: [domsecurity-backlog1])

Attachments

(1 file)

Attached file svger.html
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce:

Open PoC attached. (will close the tab you open it in, make sure you have other tabs open otherwise whole browser shuts down)

I am abusing the behavior where if you open a new tab pointed to data:text/html , firefox will now close that tab due to the relatively recent prevention of top level navigation of data uri.


Actual results:

If you open the PoC within the one and only tab, whole browser closes.


Expected results:

If you do window.close it will be prevented, so I'm sure this is not intended. The only real risk I can see is that somehow convince a user to set home page to our PoC and it will prevent them from accessing the browser.
Seems like we could maybe load about:blank instead. What do other browsers do in this case?

Christoph, can you or other people working on toplevel data: restrictions work on this?
Flags: needinfo?(qab)
Flags: needinfo?(ckerschb)
(In reply to :Gijs from comment #1)
> Christoph, can you or other people working on toplevel data: restrictions
> work on this?

Yeah. I think loading about:blank in that case seems reasonable. Smaug, do you agree? I suppose we could check if the current window and the opener window are the same within MaybeCloseWindowHelper::MaybeCloseWindow(), right?
Flags: needinfo?(ckerschb) → needinfo?(bugs)
(In reply to :Gijs from comment #1)
> Seems like we could maybe load about:blank instead. What do other browsers
> do in this case?
> 
> Christoph, can you or other people working on toplevel data: restrictions
> work on this?

Meta refresh used in the original PoC can be replaced with a location header instead with the same effect. 

Say 'redir.php' redirects to a data:text/html, url. Here are some behaviors from different browsers.


On Firefox (latest nightly checked): 
- If you visit 'redir.php' directly (typing it into address bar or clicking on anchor tag directly) an error is thrown (Navigation to toplevel data: URI not allowed...)  and you are in the same document you initiated the navigation in.

- If you open a new tab using open('redir.php','_blank'), a new tab appears then quickly closes.

- If you right click an anchor tag and open new tab (or clicking whilst holding CTRL) a new tab will appear showing 'localhost/redir.php' in the addressbar but the document is empty (cant open context menu when right clicking within document) with logs showing the error: [JavaScript Warning: "Navigation to toplevel data: URI not allowed (Blocked loading of: “data:text/html,a”)" {file: "about:blank" line: 0}]

Same tests as above but on Chrome:

- 'ERR_UNSAFE_REDIRECT' error page is shown in all cases including the main PoC from comment 0.


As far as I know, Edge doesn't allow data uri's at all on top (except a few tricks using flash), keep getting hit with '
Hmm, we can't reach this page.' error.


Seems there is something funky only when using open('redir.php','_self'). Perhaps we could throw about:neterror?e=malformedURI in some/all of the cases?
Flags: needinfo?(qab)
Hrmm seems like redirection isn't needed.

open('data:text/html,','_self');// is enough
yeah, neterror might make more sense than about:blank
Flags: needinfo?(bugs)
Christoph, where does this code live? I might be able to help if it's a straightforward fix. Either way, doesn't seem like a frontend issue.
Group: firefox-core-security → dom-core-security
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Security
Ever confirmed: true
Flags: needinfo?(ckerschb)
Product: Firefox → Core
(In reply to :Gijs (he/him) from comment #6)
> Christoph, where does this code live? I might be able to help if it's a
> straightforward fix. Either way, doesn't seem like a frontend issue.

Thanks Gijs, what you are looking for is AllowTopLevelNavigationToDataURI, see:
https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDSURIContentListener.cpp#154
Flags: needinfo?(ckerschb)
Blocks: eviltraps
Group: dom-core-security
Keywords: sec-low
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Flags: needinfo?(gijskruitbosch+bugs)
I looked at showing a neterror, but we then display the data URI as the toplevel URI anyway, so the user can hit enter in the URL bar. I'm sure someone cleverer than me can come up with a way to clickjack that, esp. if we end up with an error page that displays "try again".

I'm not sure if this is a problem. If we think giving access to the original URI this way is problematic then we should use a different, neutral data URI, like

data:text/html,Uh oh, wrong link!

or whatever, and then if the user manages to display that they might be confused but it's the website's fault, so I'm pretty OK with that.

Christoph, thoughts? If you think we should use a different URI, I'm not sure what the best way of dealing with that is... maybe it's enough to pass a channel to the new URI to DisplayLoadError(), but I'm not sure.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(ckerschb)
(In reply to :Gijs (he/him) from comment #8)
> Christoph, thoughts? If you think we should use a different URI, I'm not
> sure what the best way of dealing with that is... maybe it's enough to pass
> a channel to the new URI to DisplayLoadError(), but I'm not sure.

I would be fine with displaying that URI, but maybe we are thinking way too complicated for this corner case. Can we somehow detect how many tabs are open when we execute nsDSURIContentListener::DoContent()? If so, then we just don't call MaybeCloseWindow() if there is only one tab open, which would prevent closing the entire browser.

I think we should just map the behavior of window.location which in my opinion is the same as window.open('data:', '_self'), more or less.
Flags: needinfo?(ckerschb)
Re-needinfo'ing for comment #9.
Flags: needinfo?(gijskruitbosch+bugs)
You need to log in before you can comment on or make changes to this bug.