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

Status: RESOLVED FIXED

(External Software Affecting Firefox :: Other, defect, P1)

Description

[[:philipp]]

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?

Comment 2

[[:philipp]]

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 %

Comment 3

[Julien Cristau [:jcristau] (back April 22)]

We blocked 57.0.1 rollout while we assess this issue.

Comment 4

[Thomas Siebert]

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.

Comment 5

[[:philipp]]

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."

Comment 6

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

For now, the option we are considering is to block all g data DLLs.

Comment 8

[Tom Grabowski [:TomGrab]]

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.

Comment 9

[Julien Cristau [:jcristau] (back April 22)]

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.

Comment 10

[Marco Castelluccio [:marco]]

(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?

Comment 11

[[:philipp]]

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

[u554753]

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.

Comment 13

[[:philipp]]

the .dll should be placed under %COMMONFILES%\G DATA\AVKProxy

Comment 14

[u554753]

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

Comment 15

[u554753]

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.

Comment 16

[Jim Mathies [:jimm]]

Julien, assigning to you since you've been diagnosing and pushing to try. Please update if this is incorrect.

Comment 17

[u554753]

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.

Comment 18

[Jim Mathies [:jimm]]

Attachment: try patch

Comment 19

[Jim Mathies [:jimm]]

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.

Comment 20

[Jim Mathies [:jimm]]

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.

Comment 21

[Jim Mathies [:jimm]]

https://crash-stats.mozilla.com/search/?signature=~banksafe&product=Firefox&version=57.0.1&date=%3E%3D2017-11-23T17%3A25%3A38.000Z&date=%3C2017-11-30T17%3A25%3A38.000Z&_sort=-date&_facets=signature&_facets=platform_pretty_version&_facets=uptime&_facets=accessibility&_facets=cpu_arch&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-cpu_arch

- only happens on 64-bit
- not related to a11y
- all versions of windows
- startup crash

I guess we only need to block the 64 bit dll.

Comment 22

[Jim Mathies [:jimm]]

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.

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 24

[Marco Castelluccio [:marco]]

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

Comment 25

[Julien Cristau [:jcristau] (back April 22)]

Attachment: block-banksafe64.patch

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

Comment 26

[Thomas Siebert]

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.

Comment 27

[[:philipp]]

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...

Comment 28

[Jim Mathies [:jimm]]

(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 29

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8933584 [details] [diff] [review]
block-banksafe64.patch

Let's do that.

Comment 30

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-release/rev/52251e07782fe6e7cfb92ee05f0ef953dfc1bfd7

Comment 31

[Julien Cristau [:jcristau] (back April 22)]

Let's get this on 59 (and 58) as well.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e2edafd45c8
Add old versions of G DATA BankGuard .dll to Windows blocklist. r=jimm

Comment 33

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0e2edafd45c8

Comment 34

[Gerry Chang [:gchang]]

Comment on attachment 8933584 [details] [diff] [review]
block-banksafe64.patch

Block old versions of G DATA BankGuard .dll. Beta58+.

Comment 35

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/9db6ef2565dd

Comment 36

[Ciprian Georgiu, Desktop QA]

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.

Comment 37

[Sylvestre Ledru [:Sylvestre]]

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

Comment 38

[Sylvestre Ledru [:Sylvestre]]

I also added that it is windows only