Startup crash in banksafe64.dll on Firefox 57.0.1 with G Data Security Software

RESOLVED FIXED

Status

defect
P1
critical
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: philipp, Assigned: jcristau)

Tracking

({crash})

unspecified
Unspecified
Windows

Firefox Tracking Flags

(relnote-firefox 57+, firefox57blocking fixed, firefox58 fixed, firefox59 fixed)

Details

(crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

2 years ago
early crash data from the 57.0.1 release (updates were enabled a couple of hours ago) indicate that there's a new spiking startup crash with the banksafe.dll module, related to GData Security products.

this is very reminiscent of bug 1235537 which we suffered from nearly 2 years ago.

thomas, could you look into that again please?
Flags: needinfo?(thomas.siebert)
Reporter

Updated

2 years ago
Severity: normal → critical
Crash Signature: [@ banksafe64.dll@0x5554] [@ banksafe64.dll@0x64e4] [@ banksafe64.dll@0x55d4]
Keywords: crash
Duplicate of this bug: 1421990
Priority: -- → P1
Reporter

Comment 2

2 years ago
thus far in the 57.0.1 rollout this is causing 530 crash reports from 230 installations (28% of all browser crashes). we only process 10% of all incoming reports from release though...

locales are skewed towards german users (as gdata is a german-based company)
1 	de 	304 	57.25 %
2 	pl 	62 	11.68 %
3 	it 	19 	3.58 %
4 	nl 	14 	2.64 %
5 	en-us 	10 	1.88 %
Assignee

Comment 3

2 years ago
We blocked 57.0.1 rollout while we assess this issue.
See Also: → 1235537
See Also: → 1103828

Comment 4

2 years ago
The affected software version is ancient. Still, we rolled out a mitigation for this problem class via signature updates for this old software version quite some time ago. Only users not having their software and signatures updated for a long time are affected. Accordingly the number of affected users is quite low. As we already have rolled out updates for this, from our perspective there's nothing more we can do.
Flags: needinfo?(thomas.siebert)
Reporter

Comment 5

2 years ago
thomas, we also have use comments in crash reports telling us they're on the very latest available signatures (not familiar with the versioning scheme of your other products): 	

bp-e0eb8e40-42bb-47ce-b11d-61eea0171130:
"Only crashing with Antivirus enabled. Product:
G Data Endpoint Security 13.0.0.166
Signatures: 30.11.2017 15:38
Engine A: AVA 2514957
Engine B: GD 25.11012."
Flags: needinfo?(thomas.siebert)
Thomas, it is concerning because it is a new regression in 57.0.1.
The number is small because 57.0.1 has been published only a few hours ago.
As you still doing some voodoo black magic with the parsing of the binary layout? I guess your code is breaking us.
For now, the option we are considering is to block all g data DLLs.
QA is currently trying to reproduce this issue. We are using trial version of G DATA available on https://www.gdatasoftware.com/downloads   
"G DATA Total Security" seems to be the main product, so we will start there.
Flags: needinfo?(gwimberly)
Assignee

Comment 9

2 years ago
Pushed a dll blocklist patch to try (based on 57.0.1):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=61dd08bd8207322eaec96b189587606fd30f3b54

Same blocked versions as in the previous change from bug 1235537, the few crashes I've looked at were all banksafe64.dll versions < 1.2.15299.65535.
(In reply to Julien Cristau [:jcristau] from comment #9)
> Pushed a dll blocklist patch to try (based on 57.0.1):
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=61dd08bd8207322eaec96b189587606fd30f3b54
> 
> Same blocked versions as in the previous change from bug 1235537, the few
> crashes I've looked at were all banksafe64.dll versions < 1.2.15299.65535.

After midnight UTC we will know the precise affected versions.
Do we know why the blocklist change was backed out?
Reporter

Comment 11

2 years ago
when installing a spanish trial version of their 2014 product line from http://versiontelechargement.eu/gdata-es90dias/ESN_R_TRL_AutoTrial_2014_AV_8234_352.exe, i could at least verify that julien's blocklisting patch from comment #9 would be successful in getting the banksafe64.dll module v1.2.13XXX.XXX out of our process while triggering an artificial crash:

57.0.1: https://crash-stats.mozilla.com/report/index/0da77529-8e78-41f3-9059-05cfe1171130#tab-modules
try: https://crash-stats.mozilla.com/report/index/c4d77b37-ce3c-4a66-a878-237921171130#tab-modules

Comment 12

2 years ago
Following Comment 8,

Using the free version, I wasn't able to reproduce the issue. After a few installations and startups, I started poking around in my system32 folder on Windows and it doesn't seem that there's any mention of banksafe64.dll in the directory. 

Perhaps this dll may be downloaded if there's a paid version? I'm not sure. 

Any suggested STR to be able to get a version of GData that comes with this particular dll file? I am going to try the .exe in Comment 11 and see if that works for me.
Flags: needinfo?(gwimberly) → needinfo?(madperson)
Reporter

Comment 13

2 years ago
the .dll should be placed under %COMMONFILES%\G DATA\AVKProxy
Flags: needinfo?(madperson)

Comment 14

2 years ago
I see BankGuard32/BankGuard64.dll but nothing about banksafe64.dll in AVKProxy.

Comment 15

2 years ago
An update: I was able to see the banksafe.dll and banksafe64.dll using the installation from Comment 11. Will update if I can reproduce the crash.
Julien, assigning to you since you've been diagnosing and pushing to try. Please update if this is incorrect.
Assignee: nobody → jcristau

Comment 17

2 years ago
I tried 10 fresh installations of Firefox 57.0.1, 10 installations on top of existing installations, and 10 opening/closing Firefox with this anti-virus software installed, I don't seem to have any crashes or performance issues from trying any of these three scenarios. Can't reproduce the crash.
Reporter

Updated

2 years ago
Summary: Startup crash in banksafe.dll on Firefox 57.0.1 with G Data Security Software → Startup crash in banksafe64.dll on Firefox 57.0.1 with G Data Security Software
Posted patch try patch (obsolete) — Splinter Review
Comment on attachment 8933479 [details] [diff] [review]
try patch

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

::: mozglue/build/WindowsDllBlocklist.cpp
@@ +258,5 @@
>  
> +  // Old versions of G DATA BankGuard, bug 1421991
> +  { "banksafe.dll", MAKE_VERSION(1, 2, 15299, 65535) },
> +  { "banksafe32.dll", MAKE_VERSION(1, 2, 15299, 65535) },
> +  { "banksafe64.dll", MAKE_VERSION(1, 2, 15299, 65535) },

This will block any version of these files lower than 1.2.15299.*, which was determined in bug 1235537.

I've confirmed Grover is trying to reproduce with the older product and that his banksafe dll is 1.2.0.0. So in general appears we're testing the right thing, but so far we haven't reproduced. I'll take a shot at that next as well.
Typical versions found in crash stats:

1.2.13116.240
1.2.13245.251
1.2.14015.290
1.2.14064.293

Confirms these are all old revs of this software.
I'm not able to reproduce either. I'll bet this has to do with the version mismatch between the installed product comment 11 and the crashing versions showing up in crashstats. Unless we can find one of the revs of this AV software that's showing up in crashstats we might not be able to confirm.
Hi Julien, can you please put a patch up for review? We can get that reviewed by Aaron/Jim or another dev and gtb your daytime. I was hoping SoftVision could repro this in-house and that would help us build more confidence in the fix and the .2 release.
Flags: needinfo?(jcristau)
Here are the version numbers of the DLL on crash stats and their occurrence:
1.2.14265.1327 - 281
1.2.14036.226  - 143
1.2.14211.242  - 120
1.2.14064.293  -  96
1.2.13288.258  -  52
1.2.14141.238  -  43
1.2.14015.290  -  27
1.2.14287.730  -  26
1.2.15116.1273 -  24
1.2.13245.251  -  17
1.2.15041.877  -  10
1.2.15008.230  -   5
1.2.14307.1339 -   1
1.2.13353.236  -   1
Assignee

Comment 25

2 years ago
only blocking banksafe64.dll per comment 21.

Approval Request Comment
[Feature/Bug causing the regression]: unknown
[User impact if declined]: startup crashes with third party software
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: we weren't able to reproduce the crashes, but we can at least check that after installing an old version of g data firewall and 64-bit firefox, banksafe64.dll gets injected without this patch and is blocked with this patch.  And that firefox is still functional with the dll blocked.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: slightly
[Why is the change risky/not risky?]: possible side effects of blocking injection for users with that software
[String changes made/needed]: none
Attachment #8933479 - Attachment is obsolete: true
Flags: needinfo?(jcristau)
Attachment #8933584 - Flags: review?(jmathies)
Attachment #8933584 - Flags: approval-mozilla-release?

Comment 26

2 years ago
We're not generally opposed to a block of those ancient versions of banksafe64.dll. We're currently checking though if this might lead to any unforeseen reactions on our side. I'll give you an update ASAP.
Reporter

Comment 27

2 years ago
why aren't we using the chance and blocklist both the banksafe.dll & banksafe64.dll modules at this time? next time the 32bit module may start having issues with the binary layout of a new build and then we're in the same situation again...

Updated

2 years ago
Attachment #8933584 - Flags: review?(jmathies) → review+
(In reply to [:philipp] from comment #27)
> why aren't we using the chance and blocklist both the banksafe.dll &
> banksafe64.dll modules at this time? next time the 32bit module may start
> having issues with the binary layout of a new build and then we're in the
> same situation again...

Well there's no evidence that the 32-bit dll (whatever it might be) is an issue with current Firefox. Less risk basically.
Comment on attachment 8933584 [details] [diff] [review]
block-banksafe64.patch

Let's do that.
Attachment #8933584 - Flags: approval-mozilla-release? → approval-mozilla-release+
Assignee

Comment 31

2 years ago
Let's get this on 59 (and 58) as well.
Keywords: checkin-needed
Attachment #8933584 - Flags: approval-mozilla-beta?

Comment 32

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e2edafd45c8
Add old versions of G DATA BankGuard .dll to Windows blocklist. r=jimm
Keywords: checkin-needed

Comment 33

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0e2edafd45c8
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Comment on attachment 8933584 [details] [diff] [review]
block-banksafe64.patch

Block old versions of G DATA BankGuard .dll. Beta58+.
Attachment #8933584 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We also tried to reproduce this crash on 57.0.1 (20171128222554) under Windows 10 x64 and Windows 7 x64 but we did not succeed either.

The G Data Security Software version installed on this 2 machines was 24.0.1.5 downloaded from comment 11. Firefox did not crash at start up on our end after a few fresh installations, and it worked as expected with this version of antivirus installed.

If anyone has better steps to reproduce for QA please let us know in order to verify this fix.
Added to the 57.0.2 release notes with:
"Block old versions of G Data Endpoint Security for crashing Firefox on start up (bug 1421991)" as wording
I also added that it is windows only
Flags: needinfo?(thomas.siebert)
You need to log in before you can comment on or make changes to this bug.