1227873 - Firefox DoS with iframe on windows and non-e10s

Status: NEW

(Core :: DOM: Core & HTML, defect)

Description

[Abdulrahman Alqabandi]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.86 Safari/537.36

Steps to reproduce:

<iframe onerror='src="Q:"' src='Q:'></iframe>


Actual results:

DoS


Expected results:

No DoS

Comment 1

[Loic]

Why do you mean by DoS? Firefox hangs?

Comment 2

[Abdulrahman Alqabandi]

(In reply to Loic from comment #1)
> Why do you mean by DoS? Firefox hangs?

Yes, seems like firefox hangs when executing that iframe. Given that its a short payload this could be used in conjunction with a stored XSS to attack Firefox users.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I don't see a hang on that testcase (at least on Mac).  I can close the tab just fine, or navigate it to another page or whatever.  Which is what I would expect, since the error event fires async, so we're continuously running the event looop here.

Michelle, can you actually reproduce this bug as reported?  It's possible that this is Windows-specific...

Comment 5

[Abdulrahman Alqabandi]

(In reply to Boris Zbarsky [:bz] from comment #4)
> I don't see a hang on that testcase (at least on Mac).  I can close the tab
> just fine, or navigate it to another page or whatever.  Which is what I
> would expect, since the error event fires async, so we're continuously
> running the event looop here.
> 
> Michelle, can you actually reproduce this bug as reported?  It's possible
> that this is Windows-specific...

This wouldnt work on mac because it is trying to navigate to the non-existent drive 'Q:' (similar to C:)

Comment 6

[Abdulrahman Alqabandi]

Here is a mac version (I assume this will work though I havent tested on a mac, works on win still)

<iframe onerror='src="file://Q"' src='file://Q'></iframe>

'C:' on a windows system atleast is equal to 'file:///C:/' 

Does this one work for you?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Nope, nor does:

  <iframe onerror='src="file:///Q"' src='file:///Q'></iframe>

which would actually try to load a nonexistent file.  It just shows a "no such file" error page and never fires onerror, afaict.  The file://Q version just shows the same thing as file:/// and also never fires onerror.

Also, why would src="Q:" be loading the drive Q: when coming from a web page?  Were you loading your testcase from file:// on Windows or from a web page?

Comment 8

[Abdulrahman Alqabandi]

(In reply to Boris Zbarsky [:bz] from comment #7)
> Nope, nor does:
> 
>   <iframe onerror='src="file:///Q"' src='file:///Q'></iframe>
> 
> which would actually try to load a nonexistent file.  It just shows a "no
> such file" error page and never fires onerror, afaict.  The file://Q version
> just shows the same thing as file:/// and also never fires onerror.
> 
> Also, why would src="Q:" be loading the drive Q: when coming from a web
> page?  Were you loading your testcase from file:// on Windows or from a web
> page?

I tested it on locally hosted web server, could it be that the speed of loading from localhost is faster and thus creating a more severe dos?

Comment 9

[Michelle Funches - QA]

@Reporter & Boris: from Michelle

Yes - I tested in Nightly and FF 42.0 on Windows 8.1 and Windows 10. ("Connecting..." no error, timeout, DoS)

Comment 10

[Boris Zbarsky [:bzbarsky]]

> I tested it on locally hosted web server

In that case it's not loading a file:// URI.  It should be identical to loading

  data:text/html,<iframe onerror='src="Q:"' src='Q:'></iframe>

in the browser.  If you just do that, starting from a blank tab, do you also get an unresponsive browser?  That's the testcase I basically tried on Mac.  It's trying to load a URI with "Q" as the protocol.

Michelle, just to make sure I understand, when you say "DoS" you mean that you can't open other tabs, can't close the tab, can't navigate to other pages, etc?

Comment 11

[Abdulrahman Alqabandi]

(In reply to Boris Zbarsky [:bz] from comment #10)
> > I tested it on locally hosted web server
> 
> In that case it's not loading a file:// URI.  It should be identical to
> loading
> 
>   data:text/html,<iframe onerror='src="Q:"' src='Q:'></iframe>
> 
> in the browser.  If you just do that, starting from a blank tab, do you also
> get an unresponsive browser?  That's the testcase I basically tried on Mac. 
> It's trying to load a URI with "Q" as the protocol.
> 
> Michelle, just to make sure I understand, when you say "DoS" you mean that
> you can't open other tabs, can't close the tab, can't navigate to other
> pages, etc?
Ah yes protocol, that makes more sense.
For some reason this doesnt work when executed through the data: url scheme, nor does it seem to work when using websites like jsfiddle. 
I want to say its because this will only work when there is an html file uploaded with the content (instead of being injected through data or iframe)

alternatively, this bug might be exclusive to localhost and/or opening directly through file: scheme.

And please forget the Mac version, I think i mistakenly viewed a cached version after editing my html file. My bad.

I dont own any public servers right now and only have my own localhost to test on, Im going to upload a testcase on here to see if it works if uploaded to a non-local host.

Comment 12

[Abdulrahman Alqabandi]

Attachment: DoS testcase (dont open directly, might freeze)

Comment 13

[Abdulrahman Alqabandi]

Opening the above attachment on firefox directly results in the same DoS

Comment 14

[Abdulrahman Alqabandi]

Does not work on FF nightly v45.0a1

Comment 15

[Michelle Funches - QA]

From Michelle - Adding clarity: My test does not cause loss of functionality in FF. No freeze, crash, DoS or loss of any functionality. I can open and close new tabs/sites without error.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> Does not work on FF nightly v45.0a1

Hmm, ok.  It's possible that we fixed something...  Which version do you see the problem in?

Michelle, I guess I misunderstood comment 9?  You were saying that you could _not_ reproduce the bug as reported, on both nightly or Firefox 42?

Comment 17

[Abdulrahman Alqabandi]

(In reply to Boris Zbarsky [:bz] from comment #16)
> > Does not work on FF nightly v45.0a1
> 
> Hmm, ok.  It's possible that we fixed something...  Which version do you see
> the problem in?
> 
> Michelle, I guess I misunderstood comment 9?  You were saying that you could
> _not_ reproduce the bug as reported, on both nightly or Firefox 42?

It works on v42.0 using windows 8.1, and it prevents me from closing/opening tabs and cant even exist out of firefox.

Comment 18

[Loic]

FF45 doesn't hang with the testcase when e10s is enabled, if e10s is off, the hang is reproducible like FF42.

In addition (on Win 7), it didn't hang all the UI (tabs, window controls) in the past:
good=2013-03-23
bad=2013-03-24
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3825fdbcec62&tochange=0a10eca0c521

Maybe some OdinMonkey's patches are culprit.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Ah, good catch on e10s vs not.  I'd be _very_ surprised if OdinMonkey were an issue here, but nothing else in that range jumps out at me either.  :(

Comment 20

[Ben Kelly [:bkelly, not reviewing]]

I profiled a debug build on my windows machine.  There are two hot paths:

Function Name	                        Inclusive Samples %
nsInputStreamPump::OnStateStart	        27.24
nsBaseChannel::OnStartRequest	        27.24
nsDocumentOpenInfo::OnStartRequest	27.24
nsDocumentOpenInfo::DispatchContent	27.24
nsDocumentOpenInfo::TryContentListener	27.24
nsDSURIContentListener::DoContent	27.16
nsDocShell::CreateContentViewer	        27.16
nsDocShell::Embed	                25.41
nsDocShell::SetupNewViewer	        25.34
nsDocumentViewer::Init	                24.45
nsDocumentViewer::InitInternal	        24.45
nsGlobalWindow::SetNewDocument	        23.50

And:

Function Name	                                                Inclusive Samples %
nsInputStreamPump::OnStateStop	                                22.07
nsBaseChannel::OnStopRequest	                                22.07
nsUnicharStreamLoader::OnStopRequest	                        22.02
mozilla::css::SheetLoadData::OnStreamComplete	                22.02
mozilla::css::Loader::SheetComplete	                        22.02
nsXMLContentSink::StyleSheetLoaded	                        21.95
nsContentSink::StyleSheetLoaded	                                21.95
nsContentSink::StartLayout	                                21.91
PresShell::Initialize	                                        21.91
nsCSSFrameConstructor::ContentInserted	                        17.74
nsCSSFrameConstructor::ContentRangeInserted	                17.74
nsCSSFrameConstructor::ConstructDocElementFrame	                17.72
nsCSSFrameConstructor::SetUpDocElementContainingBlock	        17.27
nsCSSFrameConstructor::BeginBuildingScrollFrame	                17.21
nsCSSFrameConstructor::CreateAnonymousFrames	                17.14
nsCSSFrameConstructor::AddFrameConstructionItems	        15.42
nsCSSFrameConstructor::DoAddFrameConstructionItems	        15.12
nsCSSFrameConstructor::AddFrameConstructionItemsInternal	15.12
nsXBLService::LoadBindings	                                14.96
nsXBLBinding::InstallImplementation	                        14.36
nsXBLPrototypeBinding::InstallImplementation	                14.36
nsXBLProtoImpl::InstallImplementation	                        14.36
nsXBLProtoImpl::InitTargetObjects	                        14.26

Comment 21

[Ben Kelly [:bkelly, not reviewing]]

There is also a fair bit up time spent handling vsync runnables:

Function Name	Inclusive Samples %
nsRunnableMethodImpl<void (__cdecl mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp) __ptr64,1,mozilla::TimeStamp>::Run	14.96
nsRunnableMethodArguments<mozilla::TimeStamp>::apply<mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver,void (__cdecl mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp) __ptr64>	14.94
mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver	14.94
mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers	14.94
mozilla::RefreshDriverTimer::Tick	14.94
mozilla::RefreshDriverTimer::TickRefreshDrivers	14.92
mozilla::RefreshDriverTimer::TickDriver	14.91
nsRefreshDriver::Tick	14.91


This vsync refresh tick seems to be triggering GC's as well:

Function Name	Inclusive Samples %
nsXPConnect::NotifyDidPaint	8.83
JS::NotifyDidPaint	8.83
js::gc::GCRuntime::notifyDidPaint	8.83
js::gc::GCRuntime::gcSlice	8.83
js::gc::GCRuntime::collect	8.83
js::gc::GCRuntime::gcCycle	8.78

Comment 22

[Boris Zbarsky [:bzbarsky]]

That that all looks as expected; the question is why the UI is not responsive, since we're coming back to the event loop all the time....

Comment 23

[Abdulrahman Alqabandi]

Has this been confirmed on a non-windows OS? This might me just a windows OS problem?

Alternatively, since this behavior doesnt work when the DoS iframe tag is injected but rather only if the html file contains the iframe (hard coded/stored), it might be possible this is an issue with some sort of preloading firefox is performing?

Comment 24

[Abdulrahman Alqabandi]

Attachment: full windbg stacktrace for what its worth

Comment 25

[Ben Kelly [:bkelly, not reviewing]]

Profile from an opt build with symbols:

Function Name	Inclusive Samples %
firefox.exe	                                                                       100.00
js::jit::SetPropertyIC::update	                                                        60.00
js::jit::SetProperty	                                                                59.87
JSObject::nonNativeSetProperty	                                                        59.85
with_SetProperty	                                                                59.83
js::NativeSetProperty	                                                                59.80
SetExistingProperty	                                                                59.75
js::InvokeSetter	                                                                59.75
js::Invoke	                                                                        59.75
js::Invoke	                                                                        59.72
mozilla::dom::GenericBindingSetter	                                                59.65
mozilla::dom::HTMLImageElementBinding::set_src	                                        59.65
mozilla::dom::HTMLIFrameElement::SetAttr	                                        59.58
nsGenericHTMLFrameElement::SetAttr	                                                59.58
nsFrameLoader::LoadFrame	                                                        59.39
nsFrameLoader::LoadURI	                                                                58.31
nsDocument::InitializeFrameLoader	                                                57.23
nsContentUtils::AddScriptRunner	                                                        57.19
nsRunnableMethodImpl<void (__cdecl CameraPermissionRequest::*)(void) __ptr64,1>::Run	57.16
nsDocument::MaybeInitializeFinalizeFrameLoaders	                                        57.14
nsFrameLoader::ReallyStartLoading	                                                57.08
nsFrameLoader::ReallyStartLoadingInternal	                                        56.84
nsDocShell::LoadURI	                                                                55.97
nsDocShell::InternalLoad	                                                        55.43

With InternalLoad having two hot paths:

Function Name	Inclusive Samples %
nsDocShell::InternalLoad	55.43
nsDocShell::DisplayLoadError	31.25
nsDocShell::LoadErrorPage	26.35
nsDocShell::InternalLoad	23.00
nsDocShell::DoURILoad	        14.69
nsDocShell::DoChannelLoad	8.55

And:

Function Name	Inclusive Samples %
nsDocShell::InternalLoad	                                55.43
nsDocShell::DoURILoad	                                        16.08
NS_NewChannelInternal	                                        15.34
nsIOService::NewChannelFromURI2	                                15.28
nsIOService::NewChannelFromURIWithProxyFlags2	                15.28
nsIOService::NewChannelFromURIWithProxyFlagsInternal	        14.52
nsExternalProtocolHandler::NewChannel2	                        14.37
nsExternalProtocolHandler::HaveExternalProtocolHandler	        14.37
nsExternalHelperAppService::ExternalProtocolHandlerExists	14.20
nsExternalHelperAppService::GetProtocolHandlerInfo	         8.82

Boris, does this show anything different?  Seems like its being triggered from a property setter.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Sure, the testcase sets src over and over.

Again, the real question is whether we're starving user input events in this case, and if so why.

Comment 27

[Ben Kelly [:bkelly, not reviewing]]

What about bug 849647?  Its in the regression range.

Comment 28

[Ben Kelly [:bkelly, not reviewing]]

We clearly do something special on windows for "q:" btw.  If you type it into the URL bar on windows it tries to resolve file:///q:/.  On mac it goes to your search provider.

Maybe we bail out sooner in non-windows due to this, like at NS_NewURI() time, but on windows get further into the load process before the error is detected.

Comment 29

[Boris Zbarsky [:bzbarsky]]

> If you type it into the URL bar on windows it tries to resolve file:///q:/

That's due to URI fixup; that shouldn't be happening for the src attribute of iframes I would think.

> What about bug 849647?  Its in the regression range.

Oh, that's quite likely.  Masayuki-san, could you take a look, please?

Comment 30

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Boris Zbarsky [:bz] from comment #29)
> > If you type it into the URL bar on windows it tries to resolve file:///q:/
> 
> That's due to URI fixup; that shouldn't be happening for the src attribute
> of iframes I would think.

Well, if I change "Q:" to ":::", I still trigger the problem on windows.  On mac I get a 404 in the frame.

If I change "Q:" to ";;;" I get a 404 in the frame on both windows and mac.

It seems our URI fixup is doing something strange on windows here.

Comment 31

[Ben Kelly [:bkelly, not reviewing]]

Hmm, ignore comment 30.  I was running a.html on the file system on my windows machine, but over localhost http on mac.

Comment 32

[Emanuel Hoogeveen [:ehoogeveen]]

I can't confirm the regression range. I went to a local build just before 7ac778ed79a5 (bug 849647), and it did seem to behave slightly better (unfreezing momentarily every now and then), but I wouldn't call it responsive. Then I went back to a build at the start of the regression range (before bug 849647 landed), and the testcase locked the browser up completely again.

Comment 33

[Emanuel Hoogeveen [:ehoogeveen]]

I can reproduce the bad (unresponsive) behavior all the way back to the 2012-02-17 Nightly, which is the first Nightly on which the testcase doesn't print an error. The bad behavior seems harder to trigger on older builds (though it happened right away on the 2012-02-17 build), and the UI doesn't seem to freeze up as much (the loading animation continues) - but once the stuttering starts, the browser no longer responds to events and I have to kill it via the task manager.

So this may have gotten visibly worse because of bug 849647 and perhaps other bugs before it, but they aren't required to put the browser into an unresponsive state.

Comment 34

[Michelle Funches - QA]

@Boris: Pardon the delay - to answer your question "Michelle, I guess I misunderstood comment 9?  You were saying that you could _not_ reproduce the bug as reported, on both nightly or Firefox 42? "

Correct I could not reproduce to the level reported. I tested this on Windows 10 and Windows 8.1 applying Firefox Nightly and Firefox 42. I get the looping but it does not cause loss of functionality for me.

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

After I make Q drive as a network drive, I can reproduce this bug when I open the attachment directly (not downloaded into my PC). I checked Windows messages with Spy++, then, during freezing, any messages were not received (including mouse messages). I guess that the process waits some response from the drive. So, this shouldn't be message loop handling issue. Although, the bug fix causes making input events not having highest priority. I think that iframe shouldn't retry to access local resource immediately after it fails.