749648 - startup crash in CheckASLR (mostly correlated to [e-mail] add-on)

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-d061225d-5b41-4903-a1c0-707462120427 .
============================================================= 

Stack:

0 	xul.dll 	`anonymous namespace'::CheckASLR 	toolkit/xre/nsWindowsDllBlocklist.cpp:207
1 	xul.dll 	`anonymous namespace'::patched_LdrLoadDll 	toolkit/xre/nsWindowsDllBlocklist.cpp:454
2 	KERNELBASE.dll 	FreeLibrary 	
3 	nspr4.dll 	pr_LoadLibraryByPathname 	nsprpub/pr/src/linking/prlink.c:759


This is *extremely* explosive and now #1 in 13.* crashes, more crashes with this are listed in https://crash-stats.mozilla.com/report/list?signature=%60anonymous%20namespace%27%27%3A%3ACheckASLR%28wchar_t%20const%2A%29

It looks like our correlation reports didn't get correctly generated or hooked into the crash reports for this, possibly due to the quotation characters in the signature, but all reports I manually checked have this add-on loaded:

[e-mail] 	1.0

I guess that one tries to load a non-ASLR component and we crash with that. This is extremely suboptimal as it means those people cannot startup up their Firefox at all any more. :(

Comment 1

[Robert Kaiser]

Here's today's add-on version correlations for this crash, my suspicion was quite good:

Windows NT
  `anonymous namespace''::CheckASLR(wchar_t const*)|EXCEPTION_ACCESS_VIOLATION_READ (1655 crashes)
     98% (1623/1655) vs.  61% (1633/2660) [e-mail]
         67% (1107/1655) vs.  42% (1116/2660) 1.0
         31% (516/1655) vs.  19% (517/2660) 1.1
     20% (331/1655) vs.  13% (355/2660) [e-mail] (IDM CC, https://addons.mozilla.org/addon/6973)
          0% (8/1655) vs.   0% (9/2660) 6.9.8
          1% (12/1655) vs.   0% (12/2660) 7.3.11
          0% (7/1655) vs.   0% (7/2660) 7.3.14
          1% (16/1655) vs.   1% (16/2660) 7.3.15
          4% (68/1655) vs.   3% (68/2660) 7.3.16
          4% (61/1655) vs.   2% (61/2660) 7.3.17
         10% (159/1655) vs.   7% (182/2660) 7.3.18
     14% (225/1655) vs.   8% (225/2660) {c95a4e8e-816d-4655-8c79-d736da1adb6d}
          0% (5/1655) vs.   0% (5/2660) 10.7.7.9
          0% (7/1655) vs.   0% (7/2660) 3.10.0.455
         13% (208/1655) vs.   8% (208/2660) 3.12.0.7
          0% (5/1655) vs.   0% (5/2660) 3.12.2.3

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'll take this addon for a spin in a VM.

Comment 3

[Marcia Knous [:marcia]]

This is very explosive for a beta crash - I just looked and so far we have 6747 crashes on Windows. I am working on trying to reproduce now on Windows 7.

Comment 4

[Marcia Knous [:marcia]]

I have been having trouble finding version 1.0 which seems to be the crashier version of the two. This crash seems to be confined to Windows 7 ATM. I will continue working on this to try to get a reproducible case. So far now luck testing with Version 1.1 in a Windows 7 VM.

Comment 5

[Marcia Knous [:marcia]]

While we are still investigating I think we should consider blocklisting the two versions in the correlations, although I am not sure if that will help eradicate the startup crash.

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Marcia Knous [:marcia] from comment #5)
> While we are still investigating I think we should consider blocklisting the
> two versions in the correlations, although I am not sure if that will help
> eradicate the startup crash.

We are throttling updates to FF13b1 until we figure out the best way forward (hopefully after being able to repro). See bug 749877.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think the null checking here is broken.  We get mRealView back from the OS (and that is what might be null) but we null check mMappedView (which is a constant offset from mRealView.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Totally untested patch

Comment 9

[(no longer active)]

Comment on attachment 619242 [details] [diff] [review]
Totally untested patch

Review of attachment 619242 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, this is broken.  Thanks Kyle for figuring this out.

Comment 10

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e35a4ea3074

Comment 11

[Alex Keybl [:akeybl]]

We'll hold off on landing this on Aurora/Beta until we've made sure it looks good on m-c.

Kyle/Ehsan - are we confident this is the fix for the bug? What do we think the regressing bug is?

QA - have you had any luck finding STR?

Comment 12

[Alex Keybl [:akeybl]]

I've filed bug 750003 to blocklist the correlated add-on on FF13 in case that helps mitigate the crash. If it doesn't, no harm done since people are crashing on startup anyway.

Comment 13

[Jorge Villalobos [:jorgev] (he/him)]

The block is in place now.

Comment 14

[Marcia Knous [:marcia]]

No luck yet. On Friday I tested using Win 7, but as noted in an earlier comment I had trouble finding the 1.0 version which was the crashier version of two. I did install V 1.1 but I did not have any luck crashing yet.
 
> QA - have you had any luck finding STR?

Comment 15

[Marcia Knous [:marcia]]

When I tried to reproduce this on Friday, I was starting with Firefox 12 B6 and then moving to FF 13B1 via an update. The issue I was having was that Version 1.1 of the addon was installing as "incompatible" with Firefox 12. The same thing is happening today, although I believe I should be seeing it blocked according to Comment 13.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/mozilla-central/rev/0e35a4ea3074

Comment 17

[Scoobidiver (away)]

(In reply to Alex Keybl [:akeybl] from comment #11)
> We'll hold off on landing this on Aurora/Beta until we've made sure it looks
> good on m-c.
For that, you need reliable steps to reproduce.

Comment 18

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0

We can reproduce the crash on Windows 7 with the following STR:
1. Install Hotspot Shield (http://www.anchorfree.com/downloads/hotspot-shield/)
2. Make sure that the option to install the anchorfree add-on in browsers is enabled. Check all options from the installer.
3. Start Firefox 13 beta 1.
4. Allow the installation for the add-on (hotspot shield helper 1.1 or 1.0)

Result: crash 
https://crash-stats.mozilla.com/report/index/bp-cb6ace1c-c41e-49f2-a178-369f42120430
http://crash-stats.mozilla.com/report/index/bp-0f9b0524-b868-48c1-b887-e89292120430
http://crash-stats.mozilla.com/report/index/bp-a12c24de-0e12-406d-9a9e-cc7142120430

Following the installation of the Hotspot Shield, two add-ons are listed in firefox 13:
1. Hotspot Shield Helper 1.1 - which only seems to work with F13
2. HotspotShield Community toolbar- which is incompatible with F13 and disabled

If the same profile is used with F12, only the second add-on is listed in Add-ons Manager.
Also crashes on Nightly if i move the add-on(Hotspot Shield Helper 1.1) in the nightly extension directory -Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120429 Firefox/15.0a1
Will check with today's Nightly (which has Kyle's patch) when it will be available for Windows.

When we've start F12 directly after the installation of Hotspot and afterwards updated to F13, the crash did not occur.

Comment 19

[Marcia Knous [:marcia]]

Virgil: I was following similar STR to yours over the weekend but I could not generate the crash. Even now in a Win 7 VM, if I follow your steps Anchor Free Hotspot is disabled and I never actually get the option to allow the installation of the addon in FF 13B1.

(In reply to Virgil Dicu [:virgil] [QA] from comment #18)
> Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
>

Comment 20

[Virgil Dicu [:virgil] [QA]]

Attachment: afurladvisor add-on 1.1

I'm attaching the add-on that causes the crash here.

Marcia, on one computer we couldn't reproduce initially. We assumed it's due to the fact that we've started firefox 12 immediately after the installation of the hotspot app and afterwards upgraded to F13. 
We could reproduce afterwards by copying the add-on I've attached (extracting it  in the  mozilla extension directory in program files) on that respective machine. 

Could you try extracting it in the extensions directory and trying again? Enabling it in the profile used with F13 caused crashes on both machines we have tried with.

Comment 21

[Alex Keybl [:akeybl]]

(In reply to Virgil Dicu [:virgil] [QA] from comment #20)
> Could you try extracting it in the extensions directory and trying again?
> Enabling it in the profile used with F13 caused crashes on both machines we
> have tried with.

Can you confirm that the startup crash does not occur when the add-on is uninstalled? Does the add-on blocklist  we put in place over the weekend mitigate the crash? See [1] for details of how to perform a blocklist ping prior to update to FF13 beta 1.

[1] https://wiki.mozilla.org/Blocklisting/Testing#Forcing_a_Blocklist_Ping

Comment 22

[Virgil Dicu [:virgil] [QA]]

(In reply to Alex Keybl [:akeybl] from comment #21)
> Can you confirm that the startup crash does not occur when the add-on is
> uninstalled? 

Yes, definitely. Firefox starts normally with the add-on disabled/uninstalled (Hotspot Shield Helper).

(In reply to Alex Keybl [:akeybl] from comment #21)
> Does the add-on blocklist  we put in place over the weekend
> mitigate the crash? See [1] for details of how to perform a blocklist ping
> prior to update to FF13 beta 1.
> 
> [1] https://wiki.mozilla.org/Blocklisting/Testing#Forcing_a_Blocklist_Ping

I've modified the blocklist.url string and forced compatibility through error console, but upon restart I still see the same graphic default values, which i assume is not ok. 
Upgrading after following these steps still displays the crash with the add-on enabled. 
Upgrad

Comment 23

[Virgil Dicu [:virgil] [QA]]

I can confirm that today's Nightly (first with the current patch) works fine with the add-on enabled:
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120430 Firefox/15.0a1

Comment 24

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Does yesterday's nightly crash?

Comment 25

[Virgil Dicu [:virgil] [QA]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #24)
> Does yesterday's nightly crash?
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120429 Firefox/15.0a1

Yes. Checked with the one from the 29th.

Comment 26

[Alex Keybl [:akeybl]]

Comment on attachment 619242 [details] [diff] [review]
Totally untested patch

[Triage Comment]
Approved for Aurora 14 and Beta 13.

Comment 27

[Alex Keybl [:akeybl]]

(In reply to Virgil Dicu [:virgil] [QA] from comment #22)
> I've modified the blocklist.url string and forced compatibility through
> error console, but upon restart I still see the same graphic default values,
> which i assume is not ok. 
> Upgrading after following these steps still displays the crash with the
> add-on enabled. 

I don't believe you need to change any about:config values (it should be on production), only paste the ping line into the error console. We should try to figure out why [e-mail] isn't blocked. If we block the add-on, it sounds like we don't need to rush a beta 2 out the door.

Comment 28

[Marcia Knous [:marcia]]

Kyle: Is there any way to test this in an automated fashion?

Comment 29

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/ea8c07d977c3
http://hg.mozilla.org/releases/mozilla-beta/rev/e9ea9a71f28e

Comment 30

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We could probably construct a testcase from the addon's binary.

Comment 31

[Alex Keybl [:akeybl]]

Let's get testing around the instructions in [1] until bug 750330 is resolved (ETA today or tomorrow). If it's resolved by the blocklist addition, we won't go to build with FF13 Beta 2 early (and we'll unthrottle updates to beta 2 after 24 hours).

[1] https://etherpad.mozilla.org/GR8YBrtG7m

Comment 32

[u279076]

(In reply to Alex Keybl [:akeybl] from comment #31)
> [1] https://etherpad.mozilla.org/GR8YBrtG7m

Does not appear to be working. 

> 1. Install http://www.anchorfree.com/downloads/hotspot-shield/
> 2. Opt to install the toolbar
> 3. Start Firefox 13 beta 1.
> 4. Allow Hotspot Shield Helper installation and restart Firefox

At this point, Firefox crashes on every startup, so lets go add the blocklist.

> 5. Open C:/Program Files (x86)/Mozilla Firefox/blocklist.xml in an editor
> 6. Add the following code to the file between last </emitem> and </emitems>

<emItem  blockID="i87" os="WINNT" id="[e-mail]">
  <versionRange  minVersion="0" maxVersion="*">
    <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
      <versionRange  minVersion="13.0a1" maxVersion="*" />
    </targetApplication>
  </versionRange>
</emItem>

> 7. Save the file and start Firefox

Firefox still crashes repeatedly.

Please advise...

Comment 33

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #32)
> 5. Open C:/Program Files (x86)/Mozilla Firefox/blocklist.xml in an editor

blocklist.xml is located in the profile folder, not the application folder. I don't know if changing this other file will work as expected.

Comment 34

[u279076]

(In reply to Jorge Villalobos [:jorgev] from comment #33)
> (In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #32)
> > 5. Open C:/Program Files (x86)/Mozilla Firefox/blocklist.xml in an editor
> 
> blocklist.xml is located in the profile folder, not the application folder.
> I don't know if changing this other file will work as expected.

It's located in the Application folder for me...there is no blocklist.xml in the profile folder.

Note to my previous. If I start Firefox 13 with a new profile after step 7, Firefox does not crash and the add-on is properly disabled.

Comment 35

[Jorge Villalobos [:jorgev] (he/him)]

If you don't see blocklist.xml in your profile, you can force download it using the Error Console: https://wiki.mozilla.org/Blocklisting/Testing#Forcing_a_Blocklist_Ping. I'll add that to the Etherpad.

Comment 36

[u279076]

Modifying my instructions...

> 1. Install http://www.anchorfree.com/downloads/hotspot-shield/
> 2. Opt to install the toolbar
> 3. Start Firefox 12.0b6
> 4. Allow Hotspot Shield Helper installation and restart Firefox
> 5. Force a blocklist ping by running the following in Error Console:

Components.classes["@mozilla.org/extensions/blocklist;1"].getService(Components.interfaces.nsITimerCallback).notify(null);

> 6. Edit %appdata%/Mozilla/Firefox/Profiles/%profile%/blocklist.xml adding the following between last </emitem> and </emitems>

<emItem  blockID="i87" os="WINNT" id="[e-mail]">
  <versionRange  minVersion="0" maxVersion="*">
    <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
      <versionRange  minVersion="12.0a1" maxVersion="*" />
    </targetApplication>
  </versionRange>
</emItem>

> 7. Save the file and start Firefox

Add-on is disabled in Firefox 12.0b6

> 8. Check for and install update to Firefox 13.0b1

Firefox 13.0b1 starts with add-on blocklisted, does not crash.

Comment 37

[u279076]

Further to comment 36, if I keep minVersion="13.0a1" the add-on is installed/enabled in Firefox 12.0b6 but disabled after updating to Firefox 13.0b1 and does not crash.

Comment 38

[Alex Keybl [:akeybl]]

Given Comment 37, we will not rush a beta 2 to fix this bug - we'll instead wait for bug 750330 to be fixed (ETA tomorrow), and then wait another 24 hours to unthrottle beta 1.

Comment 39

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Marcia Knous [:marcia] from comment #28)
> Kyle: Is there any way to test this in an automated fashion?

I have filed bug 753340 so we can get this tested with Mozmill.

Comment 40

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Virgil Dicu [:virgil] [QA] from comment #20)
> Created attachment 619555 [details]
> afurladvisor add-on 1.1
> 
> I'm attaching the add-on that causes the crash here.

Virgil, this add-on misses the DLL which is necessary to reproduce this crash. The file itself is contained in the zip archive but it has HTML content inside. So how did it ever work for others to reproduce the crash here? Virgil, would you mind to give an update?

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+1]]

A test would be possible to implement via XPCShell but we really have to find out the reason why this DLL is broken. So it's important that we get this file.

Comment 42

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Minimized example file (.dll)

Kyle, I was working with Virgil today to investigate why this DLL caused a crash of Firefox. Together we were able to figure out the reason and you will be happy to hear that a XPCShell test would be perfectly doable even without having to compile the XPCOM binary!

So as I have already mentioned in comment 40 the DLL in the add-on is invalid. It's not a DLL file but an HTML document with a .dll extension. I asked Virgil to do some modifications of its content to figure out why we crash for invalid DLLs. So he replaced its content with some ASCII characters and the crash went away. So he had to trim the original file down a lot until at some point the crash stopped happening too. So in the crashing .dll we tried to replace characters and it still crashed. I asked him how many characters he can remove until the crash stops from happening. Finally he said that at least 64 characters are necessary to crash Firefox. So I checked the docs about the PE header of DLL/EXE files and it showed me that the header is exactly 64 byte in size.
 
So I think that due to an invalid header and a size of the file of >=64 bytes we fail in correctly parsing the PE header. And if the size is <63 bytes we correctly detect that this cannot be a valid DLL and abort its loading - no crash because of it.

So a possible XPCShell test only would have to use such a broken file and declare it as DLL. Once we load it we shouldn't crash.

Kyle, do you want a new bug for the test implementation?