836811 - crash in nsMixedContentBlocker::ShouldLoad

Status: RESOLVED FIXED

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

Description

[Scoobidiver (away)]

It first showed up in 21.0a1/20130131. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=677e87c11252&tochange=20bbf73921f4
It's likely a regression caused by bug 822367.

Signature 	nsMixedContentBlocker::ShouldLoad(unsigned int, nsIURI*, nsIURI*, nsISupports*, nsACString_internal const&, nsISupports*, nsIPrincipal*, short*) More Reports Search
UUID	22c7ed11-0a57-40ca-a90b-1db2a2130131
Date Processed	2013-01-31 16:28:02
Uptime	8
Last Crash	16 seconds before submission
Install Age	28.5 minutes since version was first installed.
Install Time	2013-01-31 15:59:29
Product	Firefox
Version	21.0a1
Build ID	20130131031009
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7600
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 37 stepping 5
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x0046, AdapterSubsysID: 392017aa, AdapterDriverVersion: 8.15.10.2104
D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers+ 
Processor Notes 	sp-processor09.phx1.mozilla.com_15932:2008
EMCheckCompatibility	True
Adapter Vendor ID	0x8086
Adapter Device ID	0x0046
Total Virtual Memory	2147352576
Available Virtual Memory	1727983616
System Memory Use Percentage	43
Available Page File	4653727744
Available Physical Memory	1732050944

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsMixedContentBlocker::ShouldLoad 	content/base/src/nsMixedContentBlocker.cpp:370
1 	xul.dll 	nsContentPolicy::CheckPolicy 	content/base/src/nsContentPolicy.cpp:127
2 	xul.dll 	nsContentPolicy::ShouldLoad 	content/base/src/nsContentPolicy.cpp:190
3 	xul.dll 	NS_CheckContentLoadPolicy 	obj-firefox/dist/include/nsContentPolicyUtils.h:212
4 	xul.dll 	nsContentUtils::CanLoadImage 	content/base/src/nsContentUtils.cpp:2643
5 	xul.dll 	nsImageLoadingContent::LoadImage 	content/base/src/nsImageLoadingContent.cpp:691
6 	xul.dll 	nsImageLoadingContent::LoadImage 	content/base/src/nsImageLoadingContent.cpp:627
7 	xul.dll 	mozilla::dom::HTMLImageElement::SetAttr 	content/html/content/src/HTMLImageElement.cpp:385
8 	xul.dll 	mozilla::dom::Element::SetAttribute 	content/base/src/Element.cpp:748
9 	xul.dll 	mozilla::dom::ElementBinding::setAttribute 	obj-firefox/dom/bindings/ElementBinding.cpp:240
10 	xul.dll 	mozilla::dom::ElementBinding::genericMethod 	obj-firefox/dom/bindings/ElementBinding.cpp:1991
11 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:390
12 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2376
13 	mozjs.dll 	js::types::TypeSet::addType 	js/src/jsinferinlines.h:1469
14 	mozjs.dll 	js::types::TypeScript::SetThis 	js/src/jsinferinlines.h:1043
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsMixedContentBlocker%3A%3AShouldLoad%28unsigned+int%2C+nsIURI*%2C+nsIURI*%2C+nsISupports*%2C+nsACString_internal+const%26%2C+nsISupports*%2C+nsIPrincipal*%2C+short*%29

Comment 1

[Boris Zbarsky [:bzbarsky]]

tvyas@120347
367 nsCOMPtr<nsISecureBrowserUI> SecurityUI;
tvyas@120347
368 rootShell->GetSecurityUI(getter_AddRefs(SecurityUI));
tvyas@120347
369 NS_ASSERTION(SecurityUI, "No SecurityUI from the root docShell.");
tvyas@120347
370 nsresult stateRV = SecurityUI->GetState(&State); 

Crash is on that last line, with a null-deref.  I bet that assertion fails too.

Which makes sense: there is zero requirement to have a security ui on the root.  I mean... what if the window here is page info or something?

Comment 2

[Tanvi Vyas[:tanvi]]

(In reply to Boris Zbarsky (:bz) from comment #1)
> tvyas@120347
> 367 nsCOMPtr<nsISecureBrowserUI> SecurityUI;
> tvyas@120347
> 368 rootShell->GetSecurityUI(getter_AddRefs(SecurityUI));
> tvyas@120347
> 369 NS_ASSERTION(SecurityUI, "No SecurityUI from the root docShell.");
> tvyas@120347
> 370 nsresult stateRV = SecurityUI->GetState(&State); 
> 
> Crash is on that last line, with a null-deref.  I bet that assertion fails
> too.
> 
> Which makes sense: there is zero requirement to have a security ui on the
> root.  I mean... what if the window here is page info or something?

I am looking into this.

Comment 3

[Tanvi Vyas[:tanvi]]

Trying to write a test case that reproduces the crash.  Interestingly, the urls where the crashes occurred are all http pages, which means that they should never have gotten to line 370 of nsMixedContentBlocker.cpp (unless they had https iframes).  So there might be something missing in the logic that detects if the parent is https.

The crash seems to occur after an image src is created through javascript and a image load is attempted.

Comment 4

[Tanvi Vyas[:tanvi]]

Some basic tests that try creating a image and setting the source attribute in javascript.  These tests don't crash the browser:
http://people.mozilla.com/~tvyas/crashtest.html
http://people.mozilla.com/~tvyas/crashtest2.html
https://people.mozilla.com/~tvyas/crashtest.html
https://people.mozilla.com/~tvyas/crashtest2.html

Comment 5

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

Perhaps this need to be fixed without any concrete url for testing. Need to just go through the
code and see why we end up to the unexpected situation.

Comment 6

[Tanvi Vyas[:tanvi]]

Attachment: Return if there is no Security UI v1

Smaug was able to reproduce with a xul:iframe:

http://www.pastebin.mozilla.org/2102149

Comment 7

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

Comment on attachment 708857 [details] [diff] [review]
Return if there is no Security UI v1

We need a test for this, but let's get this landed asap.

And we may want to think other approaches later.

Comment 8

[Tanvi Vyas[:tanvi]]

* Waiting for inbound to open to push to inbound.

* Also a limited push to try: https://tbpl.mozilla.org/?tree=Try&rev=1f7eb3373978

* Plus adding leave open to the whiteboard for the testcase that needs to be done for this.

Comment 9

[Tanvi Vyas[:tanvi]]

Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/91befa7c2488

Comment 10

[Virgil Dicu [:virgil] [QA]]

Whit a profile where Facebook Social API was previously enabled, I updated Nightly to Latest. 
Hit Restart to upgrade and then at first use of Latest, while sidebar was loading, this crash appeared.

Reproduced every time Facebook sidebar was loading. Not reproducible for a clean profile because I could not turn on Messenger fo Firefox, not even from about:config.

[@ nsMixedContentBlocker::ShouldLoad(unsigned int, nsIURI*, nsIURI*, nsISupports*, nsACString_internal const&, nsISupports*, nsIPrincipal*, short*) ] 

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130130080006

Comment 11

[Virgil Dicu [:virgil] [QA]]

Sorry for the User Agent from previous comment, it's from Firefox 19.0 beta 4.

This is the correct one from Latest Nightly.
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20130131 Firefox/21.0
Build ID: 20130131031009

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/91befa7c2488

Comment 13

[Scoobidiver (away)]

Crashes have stopped since 21.0a1/20130202.

Comment 14

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20130203 Firefox/21.0

Fixes the problem here as well with my old profile.

1. Open a profile with facebook social activated and password remembered
2. Wait for the sidebar to load.

Doesn't crash with the latest Nightly.

Comment 15

[Robert Kaiser]

It looks like URLs are no longer needed, but here's the top few:
15 	https://www.facebook.com/?ref=tn_tnmn
14 	about:sessionrestore
7 	http://www.facebook.com/
6 	about:blank

In the ones with lower volume of hits, there's pretty much a mix of http and https.

Comment 16

[Scoobidiver (away)]

What about the testcase to close this bug?

Comment 17

[Tanvi Vyas[:tanvi]]

Pastebin test case to reproduce this is now gone.  The code landed a long time ago without a test and the bug is closed.  Removing the needinfo flag.  If anyone feels we still need a test for this, please file another bug.