Open Bug 1424509 Opened 3 years ago Updated 3 years ago
Forcing browser to close using data: top navigation blocker
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?
(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)
Hrmm seems like redirection isn't needed. open('data:text/html,','_self');// is enough
yeah, neterror might make more sense than about:blank
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
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
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.
Re-needinfo'ing for comment #9.
You need to log in before you can comment on or make changes to this bug.