1235537 - startup crash in banksafe.dll on Firefox 43.0.3 with G Data Security Software

Status: RESOLVED FIXED

(External Software Affecting Firefox :: Other, defect)

Description

[[:philipp]]

[Tracking Requested - why for this release]:

This bug was filed from the Socorro interface and is 
report bp-448c296b-d071-44dc-adaf-cb38a2151229.
=============================================================
Crashing Thread (30)
Frame 	Module 	Signature 	Source
Ø 0 	banksafe.dll 	banksafe.dll@0x5cb9 	
Ø 1 	banksafe.dll 	banksafe.dll@0xbfc2 	
2 	kernel32.dll 	BaseThreadInitThunk 	
3 	ntdll.dll 	__RtlUserThreadStart 	
4 	ntdll.dll 	_RtlUserThreadStart

after the 43.0.3 there is a new startup crash with involvement of the banksafe.dll module which seems to be part of a legitimate german av vendor "G DATA" across all windows versions.

in early incoming 43.0.3 crash data, all signature variants containing this module are making up 43,4% of all crashes - nearly all of them startup crashes!

locale distribution is:
1 	de 	3212 	44.18 %
2 	pl 	1328 	18.26 %
3 	fr 	242 	3.33 %
4 	en-US 	158 	2.17 %
5 	ja 	147 	2.02 %

Comment 1

[[:philipp]]

affected users have reported that the crashes stop if they disable the "BankGuard Browserschutz" component in gdata's settings as a workaround (Einstellungen > AntiVirus > Webschutz).

german description: http://help.gdatasoftware.com/b2c/GDIS/2014/de/index.html?4470.htm

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Tracking as it is an important issue.

Comment 3

[andre.psp]

Sart Firefox in Compatibility Mode for XP SP3 also works without disableing Bank Guard

Comment 4

[Loic]

We have a reporter on the French support board about this issue on his computers with FF 43.0.3:
https://forums.mozfr.org/viewtopic.php?f=5&t=127722

Comment 5

[Thomas Siebert]

Hi,

unfortunately the last firefox update (43.3.0.3) triggered a bug in one of our components that could lead to a crash. Other Firefix versions are not affected.
We already started to roll out signature updates that prevent this from happening some hours ago.


Best regards,
Thomas Siebert - G DATA

Comment 6

[Sylvestre Ledru [:Sylvestre]]

(In reply to Thomas Siebert from comment #5)
> unfortunately the last firefox update (43.3.0.3) triggered a bug in one of
> our components that could lead to a crash. 
Changes in Firefox 43.0.3 being localized and very limited (only two changes), I am surprised by this, could you explain what caused the crash? Thanks

Comment 7

[Thomas Siebert]

The bug was actually not related to some logic changes in Firefox or something like that, but to an edge case in the binary layout in one of your modules that triggered a parsing bug.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

OK, I thought it was this patch which impacted you:
https://hg.mozilla.org/releases/mozilla-release/rev/362cb79ce600

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can you tell us why you're parsing the binary layout of our modules?

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Thomas can you confirm that a fix has rolled out to your users?  Are you sure it has fixed the issue?
Are there more details you can provide, such as the version number with the fix? That might be useful for our support pages.
   
I'd like to be able to turn updates back on for our users.  

Right now, though, I'm still seeing thousands of crashes from today, from users who already updated to Firefox 43.0.3.

Comment 11

[Krzysztof]

Users should update their virus databases, that will help AFAIK.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

I am wondering if we cannot blacklist banksafe.dll to block that?

Comment 13

[Thomas Siebert]

Hi,

we checked again that the fix is working and being rolled out. There is no particular software version that is needed, the users just need to update the signature databases and they're good. If the users have auto-update enabled, no user interaction is needed for that. Our support is also instructed to help customers with this problem. So from my point of view, you can turn updates back on (I didn't even knew you turned them off).

Most users obviously already have the update, if you look at the crash numbers:
2015-12-29 23.500 (https://crash-stats.mozilla.com/search/?product=Firefox&version=43.0.3&signature=~banksafe.dll&date=%3E%3D2015-12-29&date=%3C2015-12-30&_facets=signature&_facets=user_comments&_facets=platform_pretty_version&_facets=useragent_locale&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature)
2015-12-30 3.104 (https://crash-stats.mozilla.com/search/?product=Firefox&version=43.0.3&signature=~banksafe.dll&date=%3E%3D2015-12-30&date=%3C2015-12-31&_facets=signature&_facets=user_comments&_facets=platform_pretty_version&_facets=useragent_locale&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature)
2015-12-31 so far 1.403 (https://crash-stats.mozilla.com/search/?product=Firefox&version=43.0.3&signature=~banksafe.dll&date=%3E%3D2015-12-31&date=%3C2016-01-01&_facets=signature&_facets=user_comments&_facets=platform_pretty_version&_facets=useragent_locale&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature)

BTW, as this was asked: The reason that we parse your binaries is a protection called BankGuard that basically detects malicious hooks and therefore identifies information stealers in general and banking Trojans in particular. Blacklisting banksafe.dll would silently break this protection that our users rely on.


Best regards,
Thomas

Comment 14

[Robert Kaiser]

(In reply to Thomas Siebert from comment #13)
> Most users obviously already have the update, if you look at the crash
> numbers:
> 2015-12-29 23.500
> 2015-12-30 3.104
> 2015-12-31 so far 1.403

2015-12-30 had an issue with collecting crashes so we only have about 1/3 of crashes in total, so that number is unreliable(even though it may very well show an effect of this).

Comment 15

[Robert Kaiser]

Here's detailed correlation view for the banksafe.dll@0x5c99 signature:

All Crashes For Signature	All Crashes For OS	interesting-modules-with-versions
100% (14407/14407)	44% (23467/52887)	Banksafe.dll
0% (0/14407)	0% (3/52887)	1.2.13056.237
0% (0/14407)	0% (1/52887)	1.2.13116.239
0% (0/14407)	1% (433/52887)	1.2.13245.250
0% (0/14407)	3% (1397/52887)	1.2.13288.257
0% (0/14407)	0% (181/52887)	1.2.14015.289
43% (6229/14407)	12% (6232/52887)	1.2.14036.225
0% (0/14407)	1% (364/52887)	1.2.14064.293
9% (1266/14407)	2% (1266/52887)	1.2.14141.236
0% (3/14407)	0% (3/52887)	1.2.14174.241
36% (5199/14407)	10% (5199/52887)	1.2.14211.240
0% (0/14407)	2% (919/52887)	1.2.14265.1327
12% (1710/14407)	3% (1710/52887)	1.2.14287.729
0% (0/14407)	0% (17/52887)	1.2.14307.1338
0% (0/14407)	3% (1516/52887)	1.2.15008.228
0% (0/14407)	3% (1810/52887)	1.2.15041.877
0% (0/14407)	5% (2410/52887)	1.2.15116.1272
0% (0/14407)	0% (6/52887)	1.2.15251.1265

So it looks like 1.2.14287.729 is the last version we see crashes with, and the 1.2.15xxx.* versions are in quite a bit of usage but not crashing with this signature.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

OK, we should blacklist banksafe.dll with all the versions causing the crash. I prefer that we break silently G Data Security on old versions than preventing our users to use Firefox.

Comment 17

[[:philipp]]

Attachment: bug1235537.patch

presumtive blocklisting patch

in addition to comment #15, the other signatures are crashing in newer versions of the .dll as well

here is detailed module correlation data from 
2015-12-30: https://pastebin.mozilla.org/8855615
2015-12-31: https://pastebin.mozilla.org/8855616
and the only recent banksafe.dll file not correlated to crashes in those two dates is version 1.2.15251.1265 which has a very low coverage in crash data (present in 6 out of 52887 total crash reports)

so, i'm not sure how reliable this data is, to base a blocklist entry on it - also since only 43.0.3 and no other version in any channel is affected at all in crash data and comment #5 & comment #7 might indicate that it is a strange edgecase in 43.0.3 only. so maybe a new firefox release with a new version number might fix this crash anyway?

Comment 18

[Thomas Siebert]

Generally, you should bear in mind that there already is our signature update and therefore an existing solution out there, so I don't think a blacklist entry is really necessary. Nobody is prevented from using Firefox at all.

And, by the way: Adding or removing one or two lines of code with for xul.dll will most likely change the binary layout so that the crash would not occur anymore. So if you want to add the blacklist patch to a new version which also has other changes in xul.dll, this is absolutely pointless.

Comment 19

[[:philipp]]

thomas, did the gdata signature update containing the fix change the file-version of the banksafe.dll file (to 1.2.15251.1265)? so that even if we decided to go forward with the blocklisting approach, we'd target the right thing...

Comment 20

[Thomas Siebert]

Our current solution did not change the version of our DLL, because it is just a signature update and not a software update that changes a binary. If you want to go for the blocklisting approach, you need to blocklist everything up to 1.2.15300.*. We may release a software update with a higher version number in that case that includes the fix.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Thomas: looks like we'll have to block that version, so you should probably bump up your version number.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei or Florin, can anyone on your team reproduce this crash? I'm going to start a build with philipp's patch and would like for us to be able to verify the fix with that build (before we do a 43.0.4 release and turn on updates)    We would need to get an older version of the g data software to test with.

Comment 23

[[:philipp]]

Attachment: bug1235537v2.patch

new iteration on the blocklisting patch which blocks any banksafe.dll file below 1.2.15300.* as per comment #20.

Comment 24

[Benjamin Smedberg]

Comment on attachment 8703711 [details] [diff] [review]
bug1235537v2.patch

Why is this (1, 2, 15299, 9999) instead of 65535?

r+ either way, but I think 65535 would be the better choice.

Comment 25

[[:philipp]]

Attachment: bug1235537v3.patch

there was no particular reason (other than inexperience)

Comment 26

[Thomas Siebert]

To reproduce the bug, you not only need an old software version, but also an old signature version, because our existing signature updates fix this. And also as mentioned before, as soon as your xul.dll has a changed binary layout, the bug will not occur anymore even without this patch. Which would be the case if your blocklist entry or anything else you change in 43.0.4 ends up in xul.dll.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Thomas, OK, what i'm not sure of then is how exactly we can reproduce the crash for testing. Can you help us do that? Are you saying that we can't test with the versions with older signatures because whatever we download now will have a new signature?  We still have users stuck on 43.0.2 and about an equal number on 43.0.3 (many still hitting this crash).  So I do need to release 43.0.4 for Windows users, but am reluctant to do that without verification of the actual build that I'm about to do, through manual testing.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8703715 [details] [diff] [review]
bug1235537v3.patch

Needed to prevent high volume startup crash on Windows.

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-release/rev/413d833d300a 
This'll still need to land on trunk/aurora/beta, if it's needed on those branches.

Comment 30

[Thomas Siebert]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #27)
> Thomas, OK, what i'm not sure of then is how exactly we can reproduce the
> crash for testing. Can you help us do that? Are you saying that we can't
> test with the versions with older signatures because whatever we download
> now will have a new signature?  We still have users stuck on 43.0.2 and
> about an equal number on 43.0.3 (many still hitting this crash).  So I do
> need to release 43.0.4 for Windows users, but am reluctant to do that
> without verification of the actual build that I'm about to do, through
> manual testing.

What I'm saying is that if 43.0.4 contains any code changes to xul.dll, the bug will most certainly not trigger anymore even without the blocklist patch. As mentioned before, only 43.0.3 is affected because of an unlike side-edge case depending on the binary layout that never occured before and most likely will never occur in the future. If you add the blocklist patch without any actual need, you will absolutely do more harm than good because you silently disable a proactive protection technology that would work otherwise for a lot of our users that obviously don't even update our virus signatures on a regular basis and are therefore pretty susceptible to malware infections. This will effectively lead to our common users losing a lot of money to banking Trojans.

If you want to check your new version (preferably first without the blocklist patch to see if it's fixed anyways) for crashes with our old versions, I sent you a link to a version affected by the bug via mail. Note you should immediately disable all automatic updates including signature updates, as the bug will not trigger after the first signature update otherwise.

Comment 31

[Paul Silaghi, QA [:pauly]]

I managed to reproduce the startup crash on FF 43.0.3, Win 7 x64, with G Data up to date (last virus signature update: 1/5/2016).
No crashes occur in FF 43.0.4. Is there a way to test on 43.0.4 with the blocklist disabled?

Comment 32

[Robert Kaiser]

(In reply to Paul Silaghi, QA [:pauly] from comment #31)
> Is there a way to test on 43.0.4 with the blocklist disabled?

No, the blocklist is compiled in and can't be turned off as it needs to load extremely early in the loading process of Firefox to be able to do the blocking it needs to do.

Comment 33

[Thomas Siebert]

(In reply to Paul Silaghi, QA [:pauly] from comment #31)
> I managed to reproduce the startup crash on FF 43.0.3, Win 7 x64, with G
> Data up to date (last virus signature update: 1/5/2016).
> No crashes occur in FF 43.0.4. Is there a way to test on 43.0.4 with the
> blocklist disabled?

Our signature update in this case requires a system restart or at least a Windows user re-logon to take effect.
You should be able to test if the crash also disappears without the blocklist entry pretty simply. To switch the blacklist entry off, take a hex-editor and change the string "banksafe.dll" to "banksafe.dxx" in mods_glue.dll.

Is there any place where I can download the Firefox setup you are testing?

Comment 34

[Robert Kaiser]

(In reply to Thomas Siebert from comment #33)
> Is there any place where I can download the Firefox setup you are testing?

The builds for the new version are available at https://archive.mozilla.org/pub/firefox/candidates/43.0.4-candidates/build1/

Comment 35

[Paul Silaghi, QA [:pauly]]

(In reply to Thomas Siebert from comment #33)
> (In reply to Paul Silaghi, QA [:pauly] from comment #31)
> > I managed to reproduce the startup crash on FF 43.0.3, Win 7 x64, with G
> > Data up to date (last virus signature update: 1/5/2016).
> > No crashes occur in FF 43.0.4. Is there a way to test on 43.0.4 with the
> > blocklist disabled?
> 
> Our signature update in this case requires a system restart or at least a
> Windows user re-logon to take effect.
Confirmed the crash no longer occurs after updating the virus signature and restarting the OS, on FF 43.0.3.

> You should be able to test if the crash also disappears without the
> blocklist entry pretty simply. To switch the blacklist entry off, take a
> hex-editor and change the string "banksafe.dll" to "banksafe.dxx" in
> mods_glue.dll.
No crashes occurs in FF 43.0.4, both before and after the signature update, with the above .dll tweaked.

Comment 36

[Thomas Siebert]

We checked this too here, can confirm the blocklist entry is not needed when the blocklist entry is altered/removed from mozglue.dll

Comment 37

[Liz Henry (:lizzard) (relman/hg->git project)]

I have to do a build 2 anyway now from another issue. So I'll try backing out philipp's patch with the new build;  we will have to test once more tonight before the release rolls out.

Comment 38

[Liz Henry (:lizzard) (relman/hg->git project)]

Wes, can you back this out from m-r ? Sorry for the back and forth.

Comment 39

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-release/rev/f0106f2f82af

Unsure what the status flags should be after this since the patch apparently isn't needed any more...

Comment 40

[Johnny Stenback (:jst)]

Since we no longer need the youtube.com related changes that we took for 43.0.3 should we back them out too?

Comment 41

[[:philipp]]

looking at crash stats this is fixed in 43.0.4, which got released for different reasons in the end (as predicted by thomas in comment #18).

users on 43.0.3 that are still affected could download the firefox update at https://www.mozilla.org/firefox/all/ or https://archive.mozilla.org/pub/firefox/releases/43.0.4/win32/