Extension block request: Roboform

VERIFIED FIXED

Status

()

Toolkit
Blocklisting
VERIFIED FIXED
6 years ago
8 months ago

People

(Reporter: akeybl, Unassigned)

Tracking

({verified-aurora, verified-beta})

unspecified
verified-aurora, verified-beta
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox8 fixed, firefox9 fixed, firefox10 fixed, firefox11 fixed)

Details

(Whiteboard: [qa!][Read comment #123 before posting][extension] [dll] [hardblocker])

Attachments

(18 attachments, 2 obsolete attachments)

28.37 KB, image/png
Details
845 bytes, patch
johnath
: review+
Details | Diff | Splinter Review
834 bytes, patch
Details | Diff | Splinter Review
835 bytes, patch
sicking
: review+
Details | Diff | Splinter Review
200.28 KB, image/png
Details
34.53 KB, text/plain
Details
34.45 KB, text/plain
Details
34.21 KB, text/plain
Details
9.36 KB, text/plain
Details
9.59 KB, text/plain
Details
9.01 KB, text/plain
Details
14.35 KB, application/zip
Details
19.16 KB, application/zip
Details
17.37 KB, application/zip
Details
2.92 KB, patch
bsmedberg
: review-
Details | Diff | Splinter Review
4.56 KB, patch
Details | Diff | Splinter Review
11.82 KB, text/plain
Details
62 bytes, text/plain
Details
(Reporter)

Description

6 years ago
Extension name: Roboform
Extension UUID: ?
Extension versions to block: <7.6.2
Applications, versions, and platforms affected affected: FF8 and up, Windows tested, but likely affects all platforms
Block severity: hard, given the 

Homepage, AMO listing, other references and contact info: 
http://www.roboform.com/platforms/browsers/firefox
Vadim Maslov (vm@roboform.com)

Reasons:
Crashing on startup with default extension prefs. See bug 691271.
(Reporter)

Updated

6 years ago
Depends on: 691271

Comment 1

6 years ago
(In reply to Alex Keybl [:akeybl] from comment #0)
> Extension UUID: ?

{22119944-ED35-4ab1-910B-E619EA06A115} for Roboform Toolbar for Firefox, https://addons.mozilla.org/addon/750

Though it looks like this also happens for a lot of people who don't have the toolbar installed but Roboform is hooking their DLLs into Firefox, so we actually might need a DLL blocklist, which IIRC needs to be done in code. The DLLs names are rf-firefox.dll and roboform.dll, the version range fits for those.
I tested this again today in a VM and I was easily able to reproduce the crash using Roboform 7.5.2. If the Firefox process is attached to Roboform you get a startup crash until you go into the options an unattached the Roboform process.

The most recent version on their site is 7.6.3. I need to check to make sure that version is not causing any crashes. This will be tedious unless we do a backend query on Socorro.

I assume for a dll block we would need something like the following for each version of Roboform below 7.6.3:

rf-firefox.dll 	7.5.2.0 	7EA6667FE7B948C2B40E2849103E39612 	rf-firefox.pdb
roboform.dll 	7.5.2.0 	73136D6790324670BFFF2E8BE79B248E2 	RoboForm.pdb

For volume, the three signatures in Bug 691271 account for over 18K in crashes across all Firefox versions in the last week.
http://www.oldapps.com/roboform.php is a good resource for listing all of the old versions.

Comment 4

6 years ago
Marcia, if you have the roboform crash, can you start in safe mode?
Yes, in my VM testing it crashes even if you start in safe mode. You get the safe mode dialog, but right after that you crash.

(In reply to [:Cww] from comment #4)
> Marcia, if you have the roboform crash, can you start in safe mode?

Comment 6

6 years ago
Just for clarification, is this only an issue with Firefox 8? e.g. will we be blocking all versions through 7.5.2.0 for Firefox 8.0 only?
The lion's share of crashes seem to affect Firefox 8, this is looking at volume over the last week for the three signatures:

DbgBreakPoint - 11263 crashes in Firefox 8, 30 in 7.0.1
DebugBreak - 5118 crashes in Firefox 8, 37 in 7.0.1
kernelbase.dll@0x31f66 - 592 in Firefox 8, 2 in 7.0.1

But these crashes do affect versions up to 10 from what I can see.

Comment 8

6 years ago
Ok, could you see if you can replicate the crash with the extension blocked?

We can ask Wil to stage a block, but if you can test with the following entry added to the profile blocklist.xml, that'd be a good start to determine if we do need to block the DLL as well. I'd like to check that first, as we'll need to block it in the app (or ask roboform to respin and include a version string in the dll description, as the file version can't be checked iirc)

BL entry would look something akin to the following, assuming we're hard blocking anything before 7.6.3 for Firefox 8 and up:

<emItem id="{22119944-ED35-4ab1-910B-E619EA06A115}">
  <versionRange minVersion="0.1" maxVersion="7.6.2">
	<targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
	  <versionRange minVersion="8.0a1" maxVersion="*"/>
	</targetApplication>
  </versionRange>
</emItem>
Bug 702321 has the information about the latest version as well as the top versions implicated in the crash.
Created attachment 574428 [details]
Screenshot of Browser Integration piece

I created the xml file in Comment 8 and added it to my profile.

If both boxes are checked in the attached screenshot, Firefox still crashes on startup.

The only way I can get Firefox to start is if I uncheck the second box "Attach Roboform to Firefox...". I am testing with version 7.5.2.0 of Roboform in a Win XP VM.
I see the same thing as Marcia: adding the entry in comment #8 to the blocklist.xml does nothing to prevent the crash in Fx8 with version 7.5.2 of the addon, whether I add the entry in the existing file in the installation directory or adding a blocklist.xml in the user's profile folder.
Blocks: 701537
(In reply to juan becerra [:juanb] from comment #11)
> I see the same thing as Marcia: adding the entry in comment #8 to the
> blocklist.xml does nothing to prevent the crash in Fx8 with version 7.5.2 of
> the addon, whether I add the entry in the existing file in the installation
> directory or adding a blocklist.xml in the user's profile folder.

Exactly what method are you using to test this? Just editing the blocklist.xml file in the profile won't actually have any effect unless you also do something like deleting extensions.ini and extensions.sqlite at the same time.

Comment 13

6 years ago
Created attachment 574474 [details] [diff] [review]
roboform dll block for toolkit/xre/nsWindowsDllBlocklist.cpp

If a DLL block is required, here's the patch that needs to be applied and tested to toolkit/xre/nsWindowsDllBlocklist.cpp (apologies, I've been waiting for my repo to update, so can't submit a try build yet, and I have to leave for a couple hours). Per comment #12, please re-verify whether the blocklist works or not. If it does not, then this is the next step, and johnath will need to review.
Attachment #574474 - Flags: feedback?(johnath)
(In reply to Dave Townsend (:Mossop) from comment #12)
> (In reply to juan becerra [:juanb] from comment #11)
> > I see the same thing as Marcia: adding the entry in comment #8 to the
> > blocklist.xml does nothing to prevent the crash in Fx8 with version 7.5.2 of
> > the addon, whether I add the entry in the existing file in the installation
> > directory or adding a blocklist.xml in the user's profile folder.
> 
> Exactly what method are you using to test this? Just editing the
> blocklist.xml file in the profile won't actually have any effect unless you
> also do something like deleting extensions.ini and extensions.sqlite at the
> same time.

I edited and added the entry in the file in the installation folder. I then copied that to the profile folder. It didn't have any effect. I also deleted the extensions.ini and extension.sqlite, and that had no effect.

Could someone submit a try build with the proposed patch so we can try that?
(Reporter)

Comment 15

6 years ago
Let's kick off the try build, but if that's going to take longer than an hour to produce, we need to either come up with a local build sooner or have somebody help QA fix testing steps (without a rebuild).
Created attachment 574490 [details] [diff] [review]
Patch for Fx8 ??

Looks like it's toolkit/xre/nsWindowsDllBlocklist.h on Fx8, rather than the .cpp.

https://tbpl.mozilla.org/?tree=Try&rev=980080381874
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nthomas@mozilla.com-980080381874
(Reporter)

Comment 17

6 years ago
Thanks Nick.

Also, if anybody has suggestions for somebody to review this bug, it'd be greatly appreciated.

Comment 18

6 years ago
Johnath and vlad have done the previous reviews, so I'd defer to Johnath if he can be found.
Comment on attachment 574474 [details] [diff] [review]
roboform dll block for toolkit/xre/nsWindowsDllBlocklist.cpp

This patch looks syntactically fine, BUT

- I haven't tested it so that would need to happen before we landed/shipped it
- Marcia uses "7.5.2.0" in comment 2 and this patch uses 7.6.2.0 - is that deliberate? (Judging from the rest of comment 2 I'd assume she just typo'd, but let's be sure)
- I'd normally insert a thing about how we don't typically DLL blocklist extensions since they can be blocked with the addon blocklist which is more user-friendly, but from the sounds of it RoboForm might be hacking around that
- the aforementioned makes me grumpy
- We don't seem quite clear yet on whether our blocklist.xml tests have affirmatively ruled out that option, there's back and forth in this bug about how to test it - we should be sure
- TIL that Ehsan moved the blocklist from a .h to a .cpp in bug 648581, which landed September 19. By my reckoning that makes Nick right, and we'd want his patch (against the .h file) on FF8, but Kev's patch on FF9 and up. Still, someone should double check my math, BECAUSE:
- I just got off 12 hours of air travel and 4 days of jetlag, and the thought of me in the code review path of a release is terrifying right now so let's go through this list again and be extra sure, please.
Attachment #574474 - Flags: feedback?(johnath) → review+
I was testing with that 7.5.2 version since that is a known version that crashes and I was trying to reproduce the startup crash problem. Bug 702321#c3 has the versions that are still causing crashes. Note that in that dataset there is 1 person that crashed using 7.6.2 despite the fact that Bug 691271 claims that 7.6.2 version fixed the crashing problem.

(In reply to Johnathan Nightingale [:johnath] from comment #19)
> Comment on attachment 574474 [details] [diff] [review] [diff] [details] [review]
> roboform dll block for toolkit/xre/nsWindowsDllBlocklist.cpp
> 
> This patch looks syntactically fine, BUT
> 
> - I haven't tested it so that would need to happen before we landed/shipped
> it
> - Marcia uses "7.5.2.0" in comment 2 and this patch uses 7.6.2.0 - is that
> deliberate? (Judging from the rest of comment 2 I'd assume she just typo'd,
> but let's be sure)
(Reporter)

Comment 21

6 years ago
Created attachment 574517 [details] [diff] [review]
changed the version to 7.6.1.0

Nick's patch but with an upper bound of 7.6.1.0 since we haven't been able to reproduce on 7.6.2.0.
Attachment #574517 - Flags: review?
I tested Roboform 7.6.2 just to make sure, and it does not crash when attaching to the Firefox process.
Pushed latest patch to try, at Alex's request:
 https://tbpl.mozilla.org/?tree=Try&rev=4d03924dbec6
Attachment #574517 - Flags: review? → review+
before the "go to build 8.0.1", we need to also land this fix on relbranch  GECKO80_2011110416_RELBRANCH in http://hg.mozilla.org/releases/mozilla-release.
(Reporter)

Comment 25

6 years ago
Comment on attachment 574517 [details] [diff] [review]
changed the version to 7.6.1.0

[Triage Comment]
Confirmed working - let's get this into an 8.0.1.
Attachment #574517 - Flags: approval-mozilla-release+
Comment on attachment 574517 [details] [diff] [review]
changed the version to 7.6.1.0

http://hg.mozilla.org/releases/mozilla-release/rev/6e453b4f7056

Comment 27

6 years ago
We should also be blocking the extension per comment #8 for consistency. I'd recommend a hardblock for 7.6.1 and below for Firefox 8.0a1 and above given that we're blocking the dll as well, without which Roboform won't work

Wil/Jorge, could you stage the block for testing, and we can ideally get QA finished and push it to prod this aft?

<emItem id="{22119944-ED35-4ab1-910B-E619EA06A115}">
  <versionRange minVersion="0.1" maxVersion="7.6.1">
	<targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
	  <versionRange minVersion="8.0a1" maxVersion="*"/>
	</targetApplication>
  </versionRange>
</emItem>

Updated

6 years ago
Whiteboard: [extension] → [extension] [dll] [hardblocker]

Comment 28

6 years ago
(In reply to Johnathan Nightingale [:johnath] from comment #19)
> - I'd normally insert a thing about how we don't typically DLL blocklist
> extensions since they can be blocked with the addon blocklist which is more
> user-friendly, but from the sounds of it RoboForm might be hacking around
> that
> - the aforementioned makes me grumpy


Just to make things clear, Roboform is from all I know an externally installed application that runs on the system. It just provides an add-on as well.
I added the block on staging. Please verify.

Comment 30

6 years ago
Just a reminder for blocklist testing please see:

https://wiki.mozilla.org/Blocklisting/Testing#Testing_Staged_Blocklist

Comment 31

6 years ago
In testing discovered the blocklist in place affects all versions of Firefox, not just 8.0a1 and above.

      <emItem  blockID="i45" id="{22119944-ED35-4ab1-910B-E619EA06A115}">
                        <versionRange  minVersion="0.1" maxVersion="7.6.1">
                    </versionRange>
                  </emItem>

(In reply to Jorge Villalobos [:jorgev] from comment #29)
> I added the block on staging. Please verify.
I just added the application restriction on staging (the admin interface is kinda confusing). Please test again.

Comment 33

6 years ago
Tested with Windows XP staring with Firefox 7.0.1 and Roboform 7.6.1

- On Firefox 7.0.1 Roboform was not blocked.
- Initiated update to 8.0, and got the third-party update screen
- Enabled the Roboform extension and was informed that the extension would be enabled when a compatible version was available
- Roboform 7.6.1 was hardblocked for security/stability with link to blocklist (that 404'd)

Updated to Roboform 7.6.2 and the block was lifted/roboform was enabled.

Looks good for XP, anyone else test it out? Just need to make sure we push an entry for the reason on the blocklist as well.
(In reply to Kev [:kev] Needham from comment #33)
> - Roboform 7.6.1 was hardblocked for security/stability with link to
> blocklist (that 404'd)

Was that link pointing to the production site? I can see the entry on stage: https://addons-dev.allizom.org/en-US/firefox/blocked/

BTW, I could use a second opinion on the text in that entry. I kept it very brief.
(Reporter)

Comment 35

6 years ago
If we can reproducibly confirm that 7.0.1+RF7.6.2->8.0.1 causes a startup crash, we need to

* get a crash signature if available (or confirm it is not available)
* look at rf-firefox.dll and roboform.dll and make sure that on 7.6.2 they aren't mistakenly versioned as 7.6.1 or lower
* confirm whether the updated add-on blocklist Jorge did (not the DLL blocklist) is being picked up in QA testing
  - if the add-on blocklist prevents a crash on 7.6.2 for some reason, whether our users will get the blocklist prior to updating or on 8.0.1 first launch
(Reporter)

Comment 36

6 years ago
And I just want explicit confirmation here that the same crash does not happen when repro steps are changed to update to 8.0 (instead of 8.0.1) if possible.
(Reporter)

Comment 37

6 years ago
One other possibility is that XP RF installs have differently named DLL files than on 7 (and we thus could have blocked one DLL but not the other). Can we confirm what DLLs are being installed on XP? When we know, we should also dump versions from those DLLs.

Sadly, I only have Win7/OS X to test.
On Windows XP, we've seen an issue where if you start with Fx7.0.1 and Roboform version 7.6.2 (which is not supposed to be blocklisted), and you then install 8.0.1 on top (pave over installation), Firefox doesn't start. There is no crash. This behavior is different if you use Fx8.0, in that the resulting 8.0 build launches properly.

We've reproduced this on multiple VMs and hardware.

We need someone who understands the code to help us diagnose the problem.

I'll add the information requested by Alex in commment #35, but from what we know there is no startup crash.
You can download versions of the Roboform add-on here: http://www.oldapps.com/roboform.php
If you install roboform 7.6.2, can you check what version the following two dlls have:

rf-firefox.dll
roboform.dll

I *think* you can check the version by simply right-clicking the dll file and selecting properties or info or some such.
Jonas, in version 7.6.2, the only meaningful version number in the properties dialog says 7-6-2 for roboform.dll. I couldn't find the rf-firefox.dll anywhere.

Comment 42

6 years ago
Created attachment 574727 [details]
roboform.dll 7.6.2.0

I'm seeing the hang with 7.6.2.0 with roboform.dll on Windows XP. If it's also with 7.6.3.0, then it may be an issue that doesn't have a fix currently. We could BL all versions of the roboform.dll, but then they'd have to either change the filename with the fix, or users wouldn't have access to the extension/features of the DLL until the next release.
(Reporter)

Comment 43

6 years ago
I'm about to attach DLL dumps for FF7.0.1/8.0/8.0.1 with 7.6.2 installed. I used the following command

Listdlls.exe -v firefox > ff{version}_loaded.txt

On Windows 7, at least, the DLLs are located at

C:\Program Files\Siber Systems\AI RoboForm\RoboForm.dll
C:\Program Files\Siber Systems\AI RoboForm\Firefox\rf-firefox.dll

and both have version

Version:        7.6.2.0

Nothing jumps out at me, but we need to do the same for WinXP.
(Reporter)

Comment 44

6 years ago
Created attachment 574730 [details]
FF7.0.1+RF7.6.2 loaded DLLs
(Reporter)

Comment 45

6 years ago
Created attachment 574731 [details]
FF8.0+RF7.6.2 loaded DLLs
(Reporter)

Comment 46

6 years ago
Created attachment 574732 [details]
FF8.0.1+RF7.6.2 loaded DLLs
(Reporter)

Comment 47

6 years ago
(In reply to Kev [:kev] Needham from comment #42)
> Created attachment 574727 [details]
> roboform.dll 7.6.2.0
> 
> I'm seeing the hang with 7.6.2.0 with roboform.dll on Windows XP. If it's
> also with 7.6.3.0, then it may be an issue that doesn't have a fix
> currently. We could BL all versions of the roboform.dll, but then they'd
> have to either change the filename with the fix, or users wouldn't have
> access to the extension/features of the DLL until the next release.

It's very curious that this isn't happening with FF8.0 on XP though - and it would be a shame to blocklist working versions of RoboForm.

I'd like to get a better understanding of what could cause such different behavior between FF8 and FF8.0.1 before taking action.

Comment 48

6 years ago
On XP the addon is installed to the user's profile directory, not Program Files. Not sure if that has anything to do with it, but the example I outlined has the plugin installed in c:\Documents and Settings\Administrator\Application Data\RoboForm (that is me grasping at straws)
Created attachment 574746 [details]
Fx7.0.1+Roboform7.6.2 on XP
Created attachment 574748 [details]
Fx8.0+Roboform7.6.2 on XP
Created attachment 574751 [details]
Fx8.0.1+Roboform7.6.2 on XP
(Reporter)

Comment 52

6 years ago
It doesn't appear as if we're mistakenly blocking any DLL loads on XP in any of the dumps Juan attached.

We're now going to verify the previous assumption that an add-on blocklist alone on 8.0 doesn't resolve the startup crashes in old RF versions (which would preclude the need for a DLL blocklist addition). If the assumption was wrong, we should move ahead with that change alone (no DLL blocklist addition).
Here are some testing results from the extension block:

Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
7.6.2 - Does not Block

Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
7.6.1 Does Block

Firefox 8.0. 7.6.0 - Mohammed
Does Block

Tested Same Scenario as Kev:

Tested with Windows XP starting with Firefox 7.0.1 and Roboform 7.6.1

- On Firefox 7.0.1 Roboform was not blocked. It was disabled but I could enable it.
- Updated to 8.0, and got the third-party update screen
- Enabled the Roboform extension
- Roboform 7.6.1 was hardblocked for security/stability with link to blocklist (that 404'd)

Updated to Roboform 7.6.2 and Roboform was enabled.
Juan asked me to perform a test:

1. Install Roboform 7.5.2.
2. Launch Firefox 8 (Previously I had changed the blocklist URL to the staging server).
3. Initiate the ping. Firefox is now hard blocked.
4. Close Firefox and change the Browser Integration settings to "Attach Roboform to Firefox".
5. Firefox still crashes on startup upon relaunch.
Just for some context around the extension correlation for each signature:

DbgBreakPoint:
42% (394/933) vs.   1% (731/104696) {22119944-ED35-4ab1-910B-E619EA06A115} (Roboform Toolbar for Firefox, https://addons.mozilla.org/addon/750)

DebugBreak:
38% (135/352) vs.   1% (731/104696) {22119944-ED35-4ab1-910B-E619EA06A115} (Roboform Toolbar for Firefox, https://addons.mozilla.org/addon/750)

kernelbase.dll@0x31f66:

30% (11/37) vs.   1% (731/104696) {22119944-ED35-4ab1-910B-E619EA06A115} (Roboform Toolbar for Firefox, https://addons.mozilla.org/addon/750)
(Reporter)

Comment 56

6 years ago
I'm currently confirming that the try build in https://bugzilla.mozilla.org/show_bug.cgi?id=700835#c66 includes only the fix for 700835 and is based off of 8, so that we can try to isolate the new crashers.


In the meantime, we should kick off try builds of 8.0+699134 and 8.0+700835 in order to help us isolate the issue.
(In reply to Alex Keybl [:akeybl] from comment #56)
> In the meantime, we should kick off try builds of 8.0+699134 and 8.0+700835
> in order to help us isolate the issue.

Done, pushed separately to try, with --post-to-bugzilla set to this bug from directly on top of the FIREFOX_8_0_RELEASE tag.

http://hg.mozilla.org/try/rev/c2a0303d48bf
http://hg.mozilla.org/try/rev/d45763120aad
(Reporter)

Comment 58

6 years ago
Win32 try builds appear to be available now (thanks Callek!)

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/Callek@gmail.com-c2a0303d48bf/
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/Callek@gmail.com-d45763120aad/
Firefox appears to be crashing with a stack overflow exception:

First-chance exception at 0x7c90e8e5 (ntdll.dll) in firefox.exe: 0xC00000FD: Stack overflow.

 	ntdll.dll!_RtlDebugAllocateHeap@12()  + 0xaf bytes	
 	ntdll.dll!_RtlAllocateHeapSlowly@12()  + 0x31624 bytes	
 	ntdll.dll!_RtlAllocateHeap@12()  + 0x9b48 bytes	
 	ntdll.dll!_RtlDosPathNameToNtPathName_Ustr@16()  + 0x93 bytes	
 	ntdll.dll!_RtlDoesFileExists_UstrEx@8()  + 0x1c bytes	
 	ntdll.dll!_RtlDoesFileExists_UEx@8()  + 0x27 bytes	
 	ntdll.dll!_RtlDosSearchPath_UEx@28()  + 0x1f bytes	
 	ntdll.dll!_LdrpCheckForLoadedDll@20()  + 0x17d bytes	
 	ntdll.dll!_LdrGetDllHandleEx@20()  + 0x1cb bytes	
 	ntdll.dll!_LdrGetDllHandle@16()  + 0x18 bytes	
 	kernel32.dll!_LoadLibraryExW@12()  + 0x22e bytes	
 	fshook32.dll!100320eb() 	

fshook32 is part of F-Secure. Do we know if this crash only happens with F-secure installed also?

below here the stack is unreliable, but it may involve a loop of these calls:

 	ntdll.dll!_NtQueryInformationProcess@20()  + 0xc bytes	
 	ntdll.dll!_NtSetInformationProcess@16()  + 0xc bytes	
 	version.dll!_GetFileVersionInfoSizeW@8()  + 0x31 bytes	
 	xul.dll!patched_LdrLoadDll(wchar_t * filePath, unsigned long * flags, _UNICODE_STRING * moduleFileName, void * * handle)  + 0x42486 bytes	C++

I'm going to keep debugging.
The lab machine happens to have FSecure, but I believe Waverly and Juan were reproducing the issue without FSecure on their machines.
The iloop is apparently trying to load "C:\Program Files\Siber Systems\AI RoboForm\RoboForm.dll"

Apparently the call to GetFileVersionInfoSize here is recursing.

http://hg.mozilla.org/releases/mozilla-release/file/FIREFOX_8_0_1_BUILD1/toolkit/xre/nsWindowsDllBlocklist.cpp#l180

I'm think I'm going to try uninstalling fsecure to see if I can get better stacks.
Without fsecure the stack loop is pretty straightforward:

 	xul.dll!patched_LdrLoadDll(wchar_t * filePath, unsigned long * flags, _UNICODE_STRING * moduleFileName, void * * handle)  + 0x42486 bytes	C++
 	kernel32.dll!_SetErrorMode@4()  + 0x32 bytes	
 	version.dll!_GetFileVersionInfoSizeW@8()  + 0x31 bytes	
>	xul.dll!patched_LdrLoadDll(wchar_t * filePath, unsigned long * flags, _UNICODE_STRING * moduleFileName, void * * handle)  + 0x42486 bytes	C++
 	kernel32.dll!_SetErrorMode@4()  + 0x32 bytes	
 	version.dll!_GetFileVersionInfoSizeW@8()  + 0x31 bytes	
 	xul.dll!patched_LdrLoadDll(wchar_t * filePath, unsigned long * flags, _UNICODE_STRING * moduleFileName, void * * handle)  + 0x42486 bytes	C++
 	kernel32.dll!_SetErrorMode@4()  + 0x32 bytes	
 	version.dll!_GetFileVersionInfoSizeW@8()  + 0x31 bytes	

Now this is still a little broken, but I basically see in the assembly that in some cases GetFileVersionInfoSizeW itself calls LoadLibraryEx. This makes me very worried about any version-specific blocklist perhaps causing this failure mode.
Yes, the first things GetFileVersionInfoSize does (on WinXP at least) is call LoadLibraryEx with LOAD_LIBRARY_AS_DATAFILE. It then uses FindResource to get the version resource.

I don't know why other versions of Windows don't show this bug, perhaps because they do something different in GetFileVersionInfoSize. In any case, we could possibly work around this problem by skipping all the blocklisting checks if the _AS_DATAFILE flags are passed.

I will next try and figure out why breakpad isn't functioning.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #63)
> I don't know why other versions of Windows don't show this bug, perhaps
> because they do something different in GetFileVersionInfoSize. In any case,
> we could possibly work around this problem by skipping all the blocklisting
> checks if the _AS_DATAFILE flags are passed.

I think we should disable blocklisting completely for all DLL loads as data files.  Patch forthcoming.
Created attachment 575000 [details]
List of dlls loaded in scenarios listed in testing matrix using Fx8+699134 try-server build.

In all cases, 7.6.1, 7.6.2, 7.6.3, if I chose not to enable the add-on, Firefox launched fine. If I enabled the third-party addons in the dialog, Firefox didn't launch in 7.6.2 and 7.6.3. In the case of 7.6.1 it launched ok, with an error about not finding some robo... file.
Created attachment 575005 [details] [diff] [review]
Don't attempt to block DLLs loaded as data files
Attachment #575005 - Flags: review?(benjamin)
Try server job based on the mozilla-release repository: https://tbpl.mozilla.org/?tree=Try&rev=2dd7ebd86902

This should give us an officially branded 8.0.1 build with my patch applied to it.

Comment 68

6 years ago
Try run for 58efa7592dcb is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=58efa7592dcb
Results (out of 2 total builds):
    exception: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-58efa7592dcb
Comment on attachment 575005 [details] [diff] [review]
Don't attempt to block DLLs loaded as data files

Here's a risk assessment for this patch, as Alex requested.

If something goes wrong with this patch, we would potentially end up not block-listing some of the DLLs that we want to blocklist.  The implications are potential crashes or security vulnerabilities.

*However*, the chance of that happening is extremely low.  DLLs loaded as data files cannot execute code (unless they are already loaded somehow, in which case we lose with or without this patch), so loading blocklisted DLLs this way should not involve any risk as far as I can tell.

We definitely need QA to confirm that the DLL blocklisting still works with this patch for the list of DLLs which we currently blocklist (both versioned and non-versioned).  I will comment on the bug when the builds are available, so that QA can start testing them.
Attachment #575005 - Flags: approval-mozilla-release?
(In reply to Mozilla RelEng Bot from comment #68)
> Try run for 58efa7592dcb is complete.
> Detailed breakdown of the results available here:
>     https://tbpl.mozilla.org/?tree=Try&rev=58efa7592dcb
> Results (out of 2 total builds):
>     exception: 2
> Builds available at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.
> com-58efa7592dcb

Please ignore this.  This was a job I pushed by mistake, and I cancelled it.

Comment 71

6 years ago
I just took a look through the blocklist. Here's the list of non-malware DLLs that define a MAKE_VERSION version, and could therefore be affected

avgrsstx.dll - Antivirus vendor AVG
vksaver.dll - VKSaver, "a program to download some media content from Russian social networking site vkontakte.ru"
accelerator.dll - speedbit video accelerator

of these, the AVG one needs more investigation. I'm going to try to find out if more recent versions of AVG use this same DLL name.
(In reply to Alex Keybl from comment #71)
> I just took a look through the blocklist. Here's the list of non-malware
> DLLs that define a MAKE_VERSION version, and could therefore be affected
> 
> avgrsstx.dll - Antivirus vendor AVG
> vksaver.dll - VKSaver, "a program to download some media content from
> Russian social networking site vkontakte.ru"
> accelerator.dll - speedbit video accelerator
> 
> of these, the AVG one needs more investigation. I'm going to try to find out
> if more recent versions of AVG use this same DLL name.

Note that our blocklisting code actually fails to blocklist vksaver.dll properly, so that shouldn't be taken against this patch.
(Reporter)

Updated

6 years ago
Depends on: 703131
(Reporter)

Comment 73

6 years ago
To confirm our understanding of this issue, and get a feel for the population currently affected, we've filed bug 703131 to get crash data related to the above DLLs.

If our understanding is correct, I would expect these DLL versions to not come up in WinXP crashes from the last 7 days, but have a non-zero number on Vista/7.
(Reporter)

Updated

6 years ago
Keywords: sec-review-needed
Attachment #575005 - Flags: review?(benjamin) → review+
I don't understand comment 73. The blocklist *works*, so we shouldn't have crashes with that DLL at all. It's just that if we have a "blocklist particualar versions of a DLL" entry and that DLL is present (in any version!), Firefox will crash on Windows XP. So I don't *think* that crash stats has any useful information here.

Comment 75

6 years ago
Ugh, and because it's a stack overflow (which has lots of other causes) it's not possible to pin it to a single signature (?)
(Reporter)

Comment 76

6 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #74)
> I don't understand comment 73. The blocklist *works*, so we shouldn't have
> crashes with that DLL at all. It's just that if we have a "blocklist
> particualar versions of a DLL" entry and that DLL is present (in any
> version!), Firefox will crash on Windows XP. So I don't *think* that crash
> stats has any useful information here.

The point of that query is to verify that

* There are 0 instances of the above DLLs in unrelated crashes on WinXP with FF7.0.1/8
* There is a non-zero number of instances of the above DLLs in unrelated crashes on Vista/Win7 with FF7.0.1/8

We'll then be able to use this data to back up our understanding of the issue.
(Reporter)

Comment 77

6 years ago
(In reply to [:Cww] from comment #75)
> Ugh, and because it's a stack overflow (which has lots of other causes) it's
> not possible to pin it to a single signature (?)

We don't have crash data for this - bsmedberg is looking into why we're not catching these startup crashes.
(Reporter)

Comment 78

6 years ago
We just got initial numbers back on the queries that should show no crash instances of XP with avgrsstx.dll - https://bugzilla.mozilla.org/show_bug.cgi?id=703131#c3

Looks like NT5.1 (older XP) does show up in the list, but NT5.2 (newer XP) does not. Please confirm if this is expected.
(In reply to Alex Keybl [:akeybl] from comment #78)
> Looks like NT5.1 (older XP) does show up in the list, but NT5.2 (newer XP)
> does not. Please confirm if this is expected.

Isn't NT 5.2 Windows 2003?
(Reporter)

Comment 80

6 years ago
I'm looking at https://en.wikipedia.org/wiki/Windows_NT#Releases to see NT versions. Looks like NT5.2 include x64 WinXP, amongst others.
(Reporter)

Comment 81

6 years ago
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-2dd7ebd86902/ - builds are now available for QA testing

Comment 82

6 years ago
Try run for c2a0303d48bf is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=c2a0303d48bf
Results (out of 275 total builds):
    success: 226
    warnings: 20
    failure: 29
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/Callek@gmail.com-c2a0303d48bf
So, I tried the builds and I'm really puzzled at this point...

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-c9ee27dad619/ is a *non-PGO* build *without* my fix.  Firefox does not crash at startup with this build, which surprises me, because I expect that it _should_.

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-c7407b6fcfee/ is a *non-PGO* build *with* my fix.  Firefox does not crash at startup with this build, but this doesn't tell us anything since the previous build doesn't crash either.

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-2dd7ebd86902/ (comment 81) is a *PGO* build *with* my fix.  Firefox does not crash at startup with this build.  This should be equivalent to the current 8.0.1 candidate builds with my fix on top.  But the above makes me doubt whether the fix is really working, or whether the issue is being bypassed because of some unknown reason.

I guess I will generate a dummy build based on https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-2dd7ebd86902/ but without my fix to see if that crashes.  If that does not crash, then I will be out of ideas on whether my fix really helps or not...
(In reply to Ehsan Akhgari [:ehsan] from comment #83)
> I guess I will generate a dummy build based on
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.
> com-2dd7ebd86902/ but without my fix to see if that crashes.  If that does
> not crash, then I will be out of ideas on whether my fix really helps or
> not...

Pushed http://tbpl.mozilla.org/?tree=Try&rev=5ab1f2b20eb4 for this.

Comment 85

6 years ago
Try run for 2dd7ebd86902 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=2dd7ebd86902
Results (out of 47 total builds):
    success: 40
    warnings: 3
    failure: 4
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-2dd7ebd86902
Results on XP using tryserver build in comment #85 (starts with addon version):
* 761, addons disabled: OK, robo disabled
* 761, addons enabled: OK, robo ok
* 762, addons disabled: OK, robo disabled
* 762, addons enabled: OK, robo ok
* 763, addons disabled: OK, robo disabled
* 763, addons enabled: OK, robo ok
* 752 (known crasher), addons disabled: crash (caught)

I have the archive of files with the loaded dlls for the scenarios above, if needed.
(Reporter)

Comment 87

6 years ago
(In reply to juan becerra [:juanb] from comment #86)
> Results on XP using tryserver build in comment #85 (starts with addon
> version):
> * 752 (known crasher), addons disabled: crash (caught)

I've confirmed that 7.5.2 has properly versioned DLLs (just in case). Looks like a new regression.

We've been using http://www.oldapps.com/roboform.php?old_roboform=6651 for the 7.5.2 download. Juan let me know that that the crash signature he got was https://crash-stats.mozilla.com/report/index/bp-eb5ec8c8-a4e2-477c-8dc3-ca6d42111116
Results using Win7:
* 761, addons disabled: OK, robo disabled
* 761, addons enabled: OK, robo ok
* 762, addons disabled: OK, robo disabled
* 762, addons enabled: OK, robo ok
* 763, addons disabled: OK, robo disabled
* 763, addons enabled: OK, robo ok
* 752, addons disabled: crash

http://crash-stats.mozilla.com/report/index/bp-b9ccd34f-52bb-4718-bd2a-79f902111116
Created attachment 575112 [details]
dlls loaded in scenarios on Win7, using trybuild in comment #85,
Created attachment 575113 [details]
dlls loaded in scenarios on XP, using trybuilds in comment #85

Comment 91

6 years ago
(In reply to Alex Keybl [:akeybl] from comment #78)
> We just got initial numbers back on the queries that should show no crash
> instances of XP with avgrsstx.dll -
> https://bugzilla.mozilla.org/show_bug.cgi?id=703131#c3
> 
> Looks like NT5.1 (older XP) does show up in the list, but NT5.2 (newer XP)
> does not. Please confirm if this is expected.

5.1 is standard 32bit XP. 5.2 is usually Windows Server 2003, but XP 64bit may report as that as well. In the end, there are _very_ few instances of 5.1 in the data as well compared to 6.x (Vista/Win7), so I guess we can safely say we have no problem there on XP.
Here's why I could not reproduce the crashes with my try server build.  The blocklist patch landed only on the relbranch and not on the default branch: http://hg.mozilla.org/releases/mozilla-release/rev/6e453b4f7056.  So, my builds, which were not based on the relbranch did not include the blocklist, and hence did not crash.

Landing patches only on relbranches and not the default branch is very confusing, and it causes us to potentially miss them if we ever create a new branch.  I went ahead and landed the patch on the default branch as well: http://hg.mozilla.org/releases/mozilla-release/rev/89f6d57bf81f

I'm currently testing a local build with the blocklisting change to see whether I can reproduce the crash.
OK, with the blocklist change in my local build, I can confirm that it crashes at startup without my fix, but it doesn't crash with my fix.  Therefore, my patch does really fix the startup crash.
Builds available for testing here: <https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-83a386f3f4fc/>
I can reproduce a crash with XP and Roboform 7.6.2 with my patch.  I'm trying to debug it...
I've got an extremely reliable STR:

1. Open about:config.
2. Open the web console (Ctrl+Shift+K)
3. Paste in the following JS code:

Components.utils.import("resource://gre/modules/ctypes.jsm"); var lib = ctypes.open("C:\\Program Files\\Siber Systems\\AI RoboForm\\Firefox\\rf-firefox.dll")
(In reply to Ehsan Akhgari [:ehsan] from comment #96)
> I've got an extremely reliable STR:
> 
> 1. Open about:config.
> 2. Open the web console (Ctrl+Shift+K)
> 3. Paste in the following JS code:
> 
> Components.utils.import("resource://gre/modules/ctypes.jsm"); var lib =
> ctypes.open("C:\\Program Files\\Siber Systems\\AI
> RoboForm\\Firefox\\rf-firefox.dll")

Except that it's not quite reliable.  Windows is really making this game harder than it needs to be!
Created attachment 575358 [details] [diff] [review]
Protect against reentrancy

Turns out that Windows doesn't call LdrLoadDll with the original flags passed to LoadLibrary(Ex).  This patch does the hard thing of actually checking for re-entrancy based on a per-thread stack of the DLL names being processed in LdrLoadDll.

This seems to fix the crash for me, I've pushed a try server job: https://tbpl.mozilla.org/?tree=Try&rev=db253b723c92
Attachment #575005 - Attachment is obsolete: true
Attachment #575005 - Flags: approval-mozilla-release?
Attachment #575358 - Flags: review?(benjamin)
Attachment #575358 - Flags: approval-mozilla-release?
Alex pinged me cause the build failed.  Seems to be a missing header.  I pushed another patch which _should_ fix it: https://tbpl.mozilla.org/?tree=Try&rev=67903485b4c2 (but I pushed it without testing because I don't have access to a Windows build machine right now).

If this fails as well, and somebody is awake, please feel free to add as many headers as it would take to make this compile and push on my behalf.  :)
I'm going to sleep now as I'm struggling with a migraine attack.  :(  For the record, I'd be fine with sicking (or somebody else knowledgeable enough to r+ this, and for this to land pending that r+ and the green light from QA.

I asked Alex to go to build with this given the above.  But I still would like bsmedberg to take a look at this, and I asked Alex that if he doesn't like this patch when he looks at this tomorrow, we should pull the plug on the release.

Sorry if my personal conditions interfered with this tonight, but I feel fairly confident in this patch and I hope that we can go to build with this.  I've asked Alex to call me in the unfortunate case that this breaks down in a way which requires my attention.  Anybody else who has my phone number is also welcome to that offer.  :)
(Reporter)

Comment 101

6 years ago
Looks like the latest build failed as well: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-67903485b4c2/try-win32/try-win32-build1561.txt.gz

Let's hold off on going to build for 8.0.1 build2 till the morning. If somebody could resolve the latest build failure and push out another try build before 8AM/9AM PT tomorrow morning, it'd be much appreciated.
(In reply to Alex Keybl [:akeybl] from comment #101)
> Looks like the latest build failed as well:
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.
> com-67903485b4c2/try-win32/try-win32-build1561.txt.gz

I think I know what the issue is, so I'm working on a additional push. Currently cloning try repo.
I've hopefully pushed a fix with Ehsan's patch. Relevant links:

https://tbpl.mozilla.org/?tree=Try&rev=d8e87b590762

Updated patch: https://hg.mozilla.org/try/rev/0e496d96de38
Comment on attachment 575358 [details] [diff] [review]
Protect against reentrancy

this should definitely not be a map to nsCAutoString for several reasons:

1) this should cause noticeable shutdown leaks of autostring objects as reported by tracerefcnt
2) autostrings are for stack values, you would want nsCString
3) dllName is in scope on the stack, why not just store the const char* directly in the map, e.g.

map<DWORD, const char*>

Is there a particular reason we can't use __declspec(thread) instead of the hand-rolled map?
Attachment #575358 - Flags: review?(benjamin) → review-
(In reply to Mark Banner (:standard8) from comment #103)
> I've hopefully pushed a fix with Ehsan's patch. Relevant links:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=d8e87b590762
> 
> Updated patch: https://hg.mozilla.org/try/rev/0e496d96de38

Builds are now available here:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bugzilla@standard8.plus.com-d8e87b590762/try-win32/
Also the TLS map needs to be protected by a lock. It looks like __declspec(thread) is not usable because this code can be loaded via LoadLibrary. I'm going to try cooking up a patch now.
Trying my revised version here: https://tbpl.mozilla.org/?tree=Try&rev=98abf936800b
Builds will be at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.com-98abf936800b

Patch is here: https://hg.mozilla.org/try/rev/c988150c6903
Created attachment 575438 [details] [diff] [review]
Revised sentinel
Attachment #575438 - Flags: review?(ehsan)
Tested using the builds in comment 105 on Windows XP with the STR after which Firefox wouldn't start with the build from Wednesday.

1. Install Firefox 7.0.1
2. Install and enable Roboform 7.6.2
3. Pave-over install Try Build 8.0.1.
4. Start Firefox.

Firefox starts normally with this build

We did not check with other versions however yet.

So I understand that other builds will be available? Should we focus on testing the other test scenarios withthe try builds to be added, as per comment 108?
Comment on attachment 575438 [details] [diff] [review]
Revised sentinel

Please remove the InitializeStatics call from the ctor. The current patch reinitializes the map once for every object, so I don't think that it can actually protect against reentrancy. r=me with that.

Also as a nit, I would add back some of the comments that I had about why we're rolling our own TLS, etc.
Attachment #575438 - Flags: review?(ehsan) → review+
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #107)
> Trying my revised version here:
> https://tbpl.mozilla.org/?tree=Try&rev=98abf936800b
> Builds will be at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.
> com-98abf936800b
> 
> Patch is here: https://hg.mozilla.org/try/rev/c988150c6903

I think you should pudh another try patch with my comment addredsed for QA to test.

Comment 112

6 years ago
Hi Ehsan.
This means that we have to again wait for another build?

(In reply to Ehsan Akhgari [:ehsan] from comment #111)
> (In reply to Benjamin Smedberg  [:bsmedberg] from comment #107)
> > Trying my revised version here:
> > https://tbpl.mozilla.org/?tree=Try&rev=98abf936800b
> > Builds will be at
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.
> > com-98abf936800b
> > 
> > Patch is here: https://hg.mozilla.org/try/rev/c988150c6903
> 
> I think you should pudh another try patch with my comment addredsed for QA
> to test.
https://tbpl.mozilla.org/?tree=Try&rev=bb7ff80b8d63
and http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.com-bb7ff80b8d63

please ignore the builds from comment 107, which I think I've cancelled
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #113)
> https://tbpl.mozilla.org/?tree=Try&rev=bb7ff80b8d63
> and
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.
> com-bb7ff80b8d63
> 
> please ignore the builds from comment 107, which I think I've cancelled

These try builds are now available.
There is a bug in this patch discovered in testing (need an extra null-check). Yet more new builds:

https://tbpl.mozilla.org/?tree=Try&rev=e55c5b3c6a74
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.com-e55c5b3c6a74
(Reporter)

Comment 116

6 years ago
The build is now available for QA testing: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bsmedberg@mozilla.com-e55c5b3c6a74/try-win32/
We've tested this on XP and Win7 environments, with add-on versions 7.6.3, 7.6.2, 7.6.1, 7.5.2. We no longer get startup crashes.

The only glitch I noticed was that Process Explorer would report the roboform.dll as being loaded immediately after launch in the case where I had installed add-on version 7.5.2. If I looked later, the dll wasn't there.

Aside from that I think this trybuild looks good. You can see the results at the bottom: https://etherpad.mozilla.org/Roboform-Blocklist-Testing
(Reporter)

Comment 118

6 years ago
Comment on attachment 575358 [details] [diff] [review]
Protect against reentrancy

Fantastic news - let's make sure both of these patches are on all branches (including mozilla-release) as soon as possible. Our go to build is Sunday afternoon to allow this some time to bake on the nightly, so if this needs any more testing, please go for it.
Attachment #575358 - Flags: approval-mozilla-release?
Attachment #575358 - Flags: approval-mozilla-release+
Attachment #575358 - Flags: approval-mozilla-beta+
Attachment #575358 - Flags: approval-mozilla-aurora+
(Reporter)

Updated

6 years ago
Attachment #575358 - Flags: approval-mozilla-release+
Attachment #575358 - Flags: approval-mozilla-beta+
Attachment #575358 - Flags: approval-mozilla-aurora+
(Reporter)

Comment 119

6 years ago
Comment on attachment 575438 [details] [diff] [review]
Revised sentinel

The correct patch this time.
Attachment #575438 - Flags: approval-mozilla-release+
Attachment #575438 - Flags: approval-mozilla-beta+
Attachment #575438 - Flags: approval-mozilla-aurora+
Created attachment 575616 [details] [diff] [review]
Final version with sentinel

I will not have the opportunity to push this tonight, but I welcome somebody else doing it. I'll try to do it early tomorrow if necessary.
Attachment #575616 - Flags: approval-mozilla-release+
Attachment #575616 - Flags: approval-mozilla-beta+
Attachment #575616 - Flags: approval-mozilla-aurora+
Attachment #575438 - Flags: review-
Attachment #575438 - Flags: review+
Attachment #575438 - Flags: approval-mozilla-release+
Attachment #575438 - Flags: approval-mozilla-beta+
Attachment #575438 - Flags: approval-mozilla-aurora+
Attachment #575438 - Attachment is obsolete: true
Sent this (both patches) to all branches

http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?changeset=3d6859e4329e
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=9ece8c9cfebe
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=78c682982a80

For mozilla-release, the first (versioned) patched already landed, and I landed this on relbranch and default:
http://hg.mozilla.org/releases/mozilla-release/pushloghtml?changeset=919ff0e4ee9a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
status-firefox10: --- → fixed
status-firefox11: --- → fixed
status-firefox8: --- → fixed
status-firefox9: --- → fixed
Resolution: --- → FIXED
Roboform has been blocked in production, as specified in Comment #27.
******** ATTENTION: READ BEFORE COMMENTING *********

If your Roboform extension has been blocked, all you need to do in order to fix the problem is install the latest version.

Only versions 7.6.1 and lower have been blocked because they were causing many crashes for Firefox users. Versions 7.6.2 and above should install and work correctly.
Whiteboard: [extension] [dll] [hardblocker] → [Read comment #123 before posting][extension] [dll] [hardblocker]

Comment 124

6 years ago
Oh, but this is REALLY nice!!!!

The original document says
Why was it blocked?
    This add-on causes a high volume of crashes in Firefox versions 8 and above.
Who is affected?
    Users of Roboform versions 7.6.1 and lower using Firefox versions 8.0a1 and higher.

It so happens that I'm still using FF3.6.24 because there are several plug-ins that I use that don't work in later versions, and I've been happily RoboForming with v. 6.10.2 for a loooong time. Obviously. this doesn't come under the qualification of FF8+, but I've been blocked nevertheless. New versiones of RoboForm DON'T work with FF 3.x, so now I've lost my ability to password all my banks and other sites I usually visit.

Would the genius that came up with this please fix the situation? I DO need to access my bank accounts, you know ...

Comment 125

6 years ago
I have to second what PAdam just wrote. I'm currently still using FF v3.6.17 with an older version of Roboform with no issues and was very surprised to get the addon blocked after reading the version #'s effected.

Can someone please unblock the Roboform addon for the older versions of FF?!
I just verified that Firefox 8 downloads this blocklist entry:

> <emItem  blockID="i45" id="{22119944-ED35-4ab1-910B-E619EA06A115}">
>   <versionRange  minVersion="0.1" maxVersion="7.6.1">
>     <targetApplication  id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
>       <versionRange  minVersion="8.0a1" maxVersion="*" />
>     </targetApplication>
>   </versionRange>
> </emItem>

It looks like the version restriction is in place, but the last 2 comments indicate this is not correct. Can someone in QA please verify?

Comment 127

6 years ago
While I realize it's not the preferred way to run Firefox I ended up turning off the blocklist checking in about:config & as expected Roboform is working as should again.
I ended up updating FF to v3.6.24 & Roboform is still v6.10.1. Two reasons I didn't want to update FF or Roboform past these versions is that to go to Roboform v7 will cost money & going to FF v4 or higher breaks some of my extensions/addons (including needing Roboform v7).
If someone in charge of the blocklist corrects this I'll turn the blocklist checking back on but either way it was a good learning experience.

As an FYI, overall the blocklist must be working correctly because I have another machine with FF v7 & Roboform v7.5.2 and that one is still working correctly (per the versions it's suppose to effect).
(Reporter)

Comment 128

6 years ago
I just spent some time testing FF3.6.24 with RF6.10.0 (which reports an extension version in FF of 6.9.98).

I used the instructions at [1], since the error console command in [2] didn't seem to provide any feedback (or have an effect) when run on 3.6.24. Even using [1], it's not clear that the add-on blocklist was ever updated since app.update.lastUpdateTime.blocklist-background-update-timer didn't get set.

I instead edited the blocklist.xml file located in the profile directory and added the XML snippet from Comment #27. I did not see the RF toolbar disabled upon relaunch. QA verification will definitely help.


[1] https://wiki.mozilla.org/Extension_Blocklisting:Testing
[2] https://wiki.mozilla.org/Blocklisting/Testing#Testing_Staged_Blocklist

Comment 129

6 years ago
Tried to reproduce with Windows 7 x64 but with no luck. The following is what I tried:

Starting with FF 3.6.17
- Installed RF 6.9.95
  - Not compatible
- Tried RF 6.9.98, 6.10.0 (shown as 6.9.98), 7.1.1, 7.1.2 (both shown as 6.10.1) and 7.4.4 
  - All enabled properly
- Installed RF 7.5.2
  - Not compatible
- Uninstalled RF completely
- Installed RF 7.5.0
  - Not installed in FF
  - No dll/jar attached
  - Presumably not supported in this FF version but there was no mention of that during installation
- Upgraded to FF 3.6.24
  - Enabled properly
- Installed RF 6.9.98, 6.10.0 (shown as 6.9.98), 7.1.1 and 7.1.5
  - Enabled properly

Starting with FF 3.6.22
- Installed RF 6.9.0
  - Not compatible
- Installed RF 6.9.98
  - Enabled
- Installed RF 7.1.0 (shows as 6.10.1)
  - Enabled
- Uninstalled RF completely
  - RF 7.5.5
  - Not installed in FF
  - No dll/jar attache
- Uninstalled RF completely
- Installed RF 7.1.1, 7.1.3 (shows as 6.10.1) and 7.2.2
  - Enabled and got pop-up to update to 7.6.3
- Upgrade to FF 3.6.24
  - Extension enabled
- Tried RF 6.10.0 (shown as 6.9.98) then 7.1.0 (shown as 6.10.1)
 - Enabled properly

Starting with FF 3.6.24
- Installed RF 6.9.95
  - Not compatible
- Tried RF 6.10.0 (shown as 6.9.98), 7.1.0, 7.1.1, 7.1.2, 7.1.3 (all shown as 6.10.1), 7.2.2, 7.3.2, 7.4.0
  - All enabled
- Installed RF 7.5.0
  - Not compatible
- Upgraded to FF 7.0.1
  - RF enabled

Also tried other variations/combinations but still could not reproduce. Please provide more context or information to the setup and how/when the issue was experienced. For example, did you update Firefox and then experienced the issue? What exact version of Roboform were you using with which version of Firefox? What does it say for the extension in the add-ons manager? Did you have a certain version of Roboform, then updated and then you experienced the problem? We really appreciate any extra information we can get.

Comment 130

6 years ago
I've had RoboForm v6.10.1 on that particular machine for close to 2 years now. Had FF v3.6.17 on there for a while. When I went to use FF yesterday (11-19-11) there was a warning message to restart FF to due to Roboform not being compatible. Upon restarting FF Roboform was disabled & the toolbar was gone. After this I updated FF to v3.6.24 with RF still being disabled. 
As of now still if I leave the addons window open & toggle the blocklist checking enabled/disabled in about:config it states RF will be disabled/enabled.
Windows XP PRO sp3, 32bit OS

As I've stated earlier, if I just leave the blocklist disabled it works fine & I'm fine with that being I don't have that many extensions on that computer. Not sure why it can't be reproduced but as far as I'm concerned don't bother going crazy trying to fix it as I"m good with my fix for it.
PAdam, Jon, would you mind sending us the information corresponding to the Roboform extension that appears in the Extensions section in about:support?

I've tried installing version 6.10.2 by doing a google search that takes me to to a Softpedia site. When I install that and go to "About" in the taskbar Roboform icon, it does say that it is version 6.10.2, but the Firefox adapter is version 6.10.1. Other sites say that this version was released around March.

I tried editing the extension.update.interval and extension.blocklist.interval entries in about:config, and I've tried doing updates from 3.6.17 to 3.6.24, but it is only when I try updating to 8.0 through the major update offer that I get a warning about version incompatibility, and the older versions do get disabled after the jump.

I have not been able to reproduce the problem, yet.
@PAdam and @Jon: it would also be useful if you can attach to this bug your blocklist.xml file from your profile directory. There's info on how to find your profile on this page: https://support.mozilla.com/kb/Profiles

Comment 133

6 years ago
(In reply to juan becerra [:juanb] from comment #131)
> PAdam, Jon, would you mind sending us the information corresponding to the
> Roboform extension that appears in the Extensions section in about:support?

6.10.1, which corresponds to RoboForm 6.10.2

Comment 134

6 years ago
(In reply to Mohamed Dabbagh from comment #129)
> Also tried other variations/combinations but still could not reproduce.
> Please provide more context or information to the setup and how/when the
> issue was experienced. For example, did you update Firefox and then
> experienced the issue? What exact version of Roboform were you using with
> which version of Firefox? What does it say for the extension in the add-ons
> manager? Did you have a certain version of Roboform, then updated and then
> you experienced the problem? We really appreciate any extra information we
> can get.

The issue appeared suddenly on Nov19. Firefox had updated itself to 3.6.24 a couple of weeks before that, but RoboForm continued to work properly. No updates to either FF or RF were done on Nov19 or immediate days before.

Firefox is 3.6.24, RoboForm is 6.10.2 (with FF interface 6.10.1)

The addons manager shows the extension as greyed-out and does not allow any access to it.

I tried upgrading RoboForm up to 7.4.2 (the last one that works with FF 3x) with no change to the blocking-out. While going back again, found that v7.2.5 allowed temporary 'attachment' to browser (via Task Bar icon), which made the RF Toolbar appear again. As per Jon's hint to disable the blocklist checking (#130, above), it enabled me to go back to RF v6.10.2 (7.2.5 did not work properly with some sites requiring the use of IE engine).

Comment 135

6 years ago
Created attachment 575830 [details]
blocklist.xml as requested by Jorge Villalobos in #132
(In reply to PAdam from comment #135)
> Created attachment 575830 [details]
> blocklist.xml as requested by Jorge Villalobos in #132

Thanks. The blocklist file has the correct entry, with the correct version range. For some reason Firefox is failing to understand it shouldn't block. Perhaps an add-on that makes it think it has a different version number?

Comment 137

6 years ago
(In reply to Jorge Villalobos [:jorgev] from comment #136)
>For some reason Firefox is failing to understand it shouldn't block.
> Perhaps an add-on that makes it think it has a different version number?

(from InfoLister)

Last updated: Mon, 21 Nov 2011 16:02:34 GMT

User Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.24) Gecko/20111103 Firefox/3.6.24 ( .NET CLR 3.5.30729; .NET4.0E)

*** Extensions (enabled: 18, disabled: 5; total: 23)
AboutPlug 1.5 
AI Roboform Toolbar for Firefox 6.10.1 
Bookmark This Page 0.3.2 [disabled]
DivX HiQ 2.1.1.94 
DivX Plus Web Player HTML5 <video> 2.1.1.94 
Edit Cookies 0.3.8.1 
Favicon Picker 3 0.5 
FireFTP 1.0.10 
IE Tab 1.5.20090525 [disabled]
IE Tab 2 (FF 3.6+) 3.10.7.2 [disabled]
IE Tab Plus 1.2.0.13 
IE View 1.4.5.2 [disabled]
InfoLister 0.10.3 
Java Console 6.0.24 
Java Console 6.0.26 
Java Quick Starter 1.0 
Link Pad 1.6.4 
Microsoft .NET Framework Assistant 1.3.1 
Print Preview Toolbar Button 0.1 [disabled]
Saved Password Editor 2.2.5 
SQLite Manager 0.6.8 
Tab Counter 1.8.8 
Tab Mix Plus 0.3.8.6 

*** Themes (3)
Default 
Simple Green [selected]
Unofficial Netstripe 

*** Plugins
Adobe Acrobat
BrowserPlus (from Yahoo!) v2.9.8
DivX VOD Helper Plug-in
DivX Web Player
getPlusPlus for Adobe 16263
Google Earth Plugin
Google Update
IE Tab Plug-in
Java Deployment Toolkit 6.0.260.3
Java(TM) Platform SE 6 U26
Microsoft Office 2003
Microsoft® DRM
Microsoft® Windows Media Player Firefox Plugin
Mozilla Default Plug-in
QuickTime Plug-in 7.6.9
RealJukebox NS Plugin
RealPlayer Version Plugin
RealPlayer(tm) G2 LiveConnect-Enabled Plug-In (32-bit) 
Shockwave Flash
Shockwave for Director
Silverlight Plug-In
VLC Multimedia Plug-in
Winamp Application Detector
Windows Genuine Advantage
Windows Media Player Plug-in Dynamic Link Library
Windows Presentation Foundation
Yahoo Application State Plugin
Yahoo! activeX Plug-in Bridge
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0.1) Gecko/20100101 Firefox/8.0.1
Mozilla/5.0 (Windows NT 5.1; rv:8.0.1) Gecko/20100101 Firefox/8.0.1

Checked on the latest 8.0.1 build2 on Windows XP and 7 with Roboform 7.6.1, 7.6.2, 7.6.3 using the following steps: 
1. Install Firefox 7.0.1.
2. Install Roboform version.
3. Upgrade to 8.0.1 using a pave-over install.
4. Start Firefox with add-on enabled/disabled.

Firefox no longer crashes in any situation and Roboform is listed as enabled/disabled according to the option set in the Confirm Add-on screen. The only unexpected issue (also specified by Juan) is the error message received with 7.6.1, when Roboform is enabled after the upgrade an error message is displayed: "Could not initialize RoboForm add-on"

Comment 139

6 years ago
(In reply to Jorge Villalobos [:jorgev] from comment #136)
> The blocklist file has the correct entry, with the correct version
> range. For some reason Firefox is failing to understand it shouldn't block.

Just to see what would happen, I toggled extensions.blocklist.enabled back to on and lo and behold, evrything is just working as it always has (at least regarding the RoboForm plugin).

I do notice, though, that the blocklist.xml file is dated 20Nov 11:00, which is after I started to experience problems and right in the middle when I was doing RF reinstallations and toggling on and off like crazy. Could it be that RF got blocked by an erroneus blocklist.xml sent out on 19Nov and corrected by the one that now sits in my 'puter?
(In reply to PAdam from comment #139)
> Could it be that RF got blocked by an erroneus blocklist.xml sent out on
> 19Nov and corrected by the one that now sits in my 'puter?

It's possible. The blocklist tools are set up in a way that you first need to add the block and after that you set the application version limitation. It only took me a couple of minutes to finish the second step, so I didn't expect anyone to get the wrong blocklist during that window (due to caching and whatnot).

I could be that things can be done in a different order (it's the first block I've had to deal with), but we should fix those tools so that it all happens in a single transaction.
When adding a block that needs a specific app version range, I usually block the add-on with minVersion * and then add the version range and then go back and edit the * down to the actual version to make sure no one gets blocked incorrectly.

Comment 142

6 years ago
OK so I think this is the problem....

From blocklist

</emItem> -<emItem id="{22119944-ED35-4ab1-910B-E619EA06A115}" blockID="i45"> <versionRange maxVersion="7.6.1" minVersion="0.1"> </versionRange> </emItem>


From t/s info page (with blocklist enabled)....

AI Roboform Toolbar for Firefox
        6.10.1
        false
        {22119944-ED35-4ab1-910B-E619EA06A115}

My blocklist file is still from 11-19-11. Is there some way I can force it to update? I tried reinstalling FF v3.6.24 but that didn't do it. If I'm reading this correctly it's blocking all versions of RF under v7.6.2
Delete/renaming the file while Firefox isn't running should work.

Comment 144

6 years ago
I closed FF & made sure it wasn't running in the Task Manager, deleted the blocklist.xml & opened FF. FF runs but a new blocklist.xml didn't get created.

Comment 145

6 years ago
FF finally got a new blocklist this morning and RF is back to working as normal. Thanks for the help guys.
Keywords: sec-review-needed

Comment 146

6 years ago
This may result in my no longer using Firefox as my browser.  I want the ability to override your block list.  I don't mind the occasional crash if I get the functionality Roboform provides.  What is my recourse here?
(Reporter)

Comment 147

6 years ago
(In reply to Donald Delozier from comment #146)
> This may result in my no longer using Firefox as my browser.  I want the
> ability to override your block list.  I don't mind the occasional crash if I
> get the functionality Roboform provides.  What is my recourse here?

Donald - What issue are you running into? What version of Firefox are you on? This block didn't prevent the occasional crash, but instead prevents a startup crash specifically on Firefox 8 that's reproducible on each launch.
(Reporter)

Comment 148

6 years ago
Forgot to mention that if your problem is that you find RoboForm is now disabled, you should upgrade to a RoboForm version later than 7.6.1.

Comment 149

6 years ago
If you already have v7 of RF you should be able to get the newest version for free. If something is stopping you from doing that you can try disabling the list like I mentioned in comment 127 or just stick with FF v7 as I believe this block should only effect FF v8 and above.

Comment 150

6 years ago
Thank you all for the quick responses.  Normal RoboForm update processes were not identifying the updates past 7.6.1 (which is where I was).  I have since manually installed the latest version 7.6.4 and it is back.  

I appreciate the help.  It was unclear that 7.6.1 was the only real troublesome version and RoboForm did not make it clear there was a later version when you select "Check for Updates".

I'm all set.
Whiteboard: [Read comment #123 before posting][extension] [dll] [hardblocker] → [qa+][Read comment #123 before posting][extension] [dll] [hardblocker]
Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0 Beta 5 
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111208 Firefox/10.0a2
Mozilla/5.0 (Windows NT 5.1; rv:11.0a1) Gecko/20111209 Firefox/11.0a1
Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0 Beta 5
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111206 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111209 Firefox/11.0a1

Verified on Firefox 9 Beta5, Firefox Aurora, Firefox Nightly on Windows XP and 7.
Used Roboform 7.6.1 (blacklisted) 7.6.2 and 7.6.6 (allowed).

The blacklisted version is disabled in F9 and Aurora-enabling it will return an error as stated in comment 138 and https://etherpad.mozilla.org/Roboform-Blocklist-Testing
While on nightly it is blacklisted without the possibility of enabling and with a notification leading to the blocklist roboform page.

Version higher than 7.6.1 work as normal, without any crashes occuring at start-up.
Status: RESOLVED → VERIFIED
Keywords: verified-aurora, verified-beta
Whiteboard: [qa+][Read comment #123 before posting][extension] [dll] [hardblocker] → [qa!][Read comment #123 before posting][extension] [dll] [hardblocker]

Comment 152

5 years ago
Still NOT working here. :(

Firefox 8 on Win7 x64 with Roboform 7.6.7

I get this:

roboform> zbx: Can not load RoboForm add-on
Please restart browser or reinstall RoboForm

Please restart browser

Incompatible module C:\Program Files (x86)\Pale Moon\extensions\{22119944-ED35-4ab1-910B-E619EA06A115}\rf-firefox.dll


Warning: WARN addons.manager: Exception calling callback: Please restart browser

Incompatible module C:\Program Files (x86)\Pale Moon\extensions\{22119944-ED35-4ab1-910B-E619EA06A115}\rf-firefox.dll

Comment 153

5 years ago
War59312, I tried with FF8 and Roboform 7.6.7 on Windows x64 and it was not blocked for me. Did you do an upgrade to either Firefox or Roboform lately? Can you try the steps in comment #144 (try this first) or comment #139 and see if it fixes your problem? Or try with a new Firefox profile and see if you notice the same issue. This way it can be known if it's an problem with your profile specifically. Thanks!

Comment 154

5 years ago
@War59312
Are you actually using Firefox or PaleMoon? Looking at the directories of your errors you're using PaleMoon. While I realize PaleMoon is basically Firefox are all of the extensions actually supposed to be compatible? I have PaleMoon on my computer also but just the portable version & haven't tried installing any extensions in it.
Also it appears as though it's not being blocked as this thread was originally about, it appears as though it's just not starting for some other reason.

Comment 155

5 years ago
Whoops yea I was trying it in both (normal firefox and pale moon), but I got it working.

I restarted windows and deleted all files in %temp% and now it works just fine. I had already tried deleting the block list with no luck.

Figures! Thanks guys! Confirmed fixed. :)
(Assignee)

Updated

a year ago
Product: addons.mozilla.org → Toolkit

Comment 156

8 months ago
Created attachment 8787724 [details]
Reference to incorrect Mozilla hard block.

Mozilla today blocked Roboform Version 7.9.16.7 with the reference to this bug filed in 2011 for Firefox 8 and above. Certainly this is incorrect. Please unblock.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1293964 for details of the latest block - we re-used the old entry and updated the version.
You need to log in before you can comment on or make changes to this bug.