1958112 - Crash in [@ chromesafe64.dll | _tailMerge_d3dcompiler_47.dll | chromesafe64.dll | NS_TableDrivenQI]

Status: VERIFIED FIXED

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

Description

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

This is the third most frequent crash for Firefox 137.0 at this point.

The loaded module chromesafe64.dll is from Beijing Qihu Technology Co., Ltd. which offers the Qihoo 360 antivirus product.

Crash report: https://crash-stats.mozilla.org/report/index/1036cb0a-bcca-4aeb-b728-e7e370250402

Reason:

EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  chromesafe64.dll  chromesafe64.dll@0x4233e
1  chromesafe64.dll  chromesafe64.dll@0x3ab6
2  xul.dll  _tailMerge_d3dcompiler_47.dll
3  chromesafe64.dll  chromesafe64.dll@0x2f138
4  xul.dll  NS_TableDrivenQI(void*, nsID const&, void**, QITableEntry const*)  xpcom/base/nsISupportsImpl.cpp:24
4  xul.dll  mozilla::net::nsSimpleURI::QueryInterface(nsID const&, void**)  netwerk/base/nsSimpleURI.cpp:60
5  ?  @0x000007e7f2870947
6  xul.dll  _tailMerge_d3dcompiler_47.dll
7  chromesafe64.dll  chromesafe64.dll@0x3e8ff
8  xul.dll  js::gc::HeaderWord::get() const  js/src/gc/Cell.h:108

Comment 1

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

David, could you take a look at this? Based on bug 1843977 and bug 1844461, you have knowledge about the interaction.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 20 desktop browser crashes on release

For more information, please visit BugBot documentation.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox137 (release). However, the bug still isn't assigned.

:gcp, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 4

[Greg Stoll :gstoll]

Many of the comments seem to indicate this is happening when downloading a file.

Comment 5

[Yannis Juglaret [:yannis]]

(For QA: please follow this more detailed document)

I reproduced the crash as follows, on a dedicated machine:

• Install http://dl.360safe.com/inst.exe .
• Install Firefox 137.
• Open, close and relaunch Firefox.
• Navigate to https://pdfgeneratorapi.com/example-documents .
• In Bill of Lading (BOL), click “Preview”.
• In the pdfjs interface, click the “Save” button.
• In the file picker, hit Enter to actually save the file.

Expected result: The file is saved.

Actual result: Firefox crashes.

Comment 6

[Yannis Juglaret [:yannis]]

Attachment: chromesafe64.dll

Attaching chromesafe64.dll 2.0.0.40 with which I reproduced and investigated the issue, for reference

Comment 7

[Yannis Juglaret [:yannis]]

After reproducing the crash in a WinDbg time-travel trace, I reached the following conclusion. First, let us observe that the layout for mozilla::net::nsSimpleURI objects has changed in version 137 after bug 1944365. This is the layout in version 136 (for x64):

dt xul!mozilla::net::nsSimpleURI
   +0x018 mRefCnt          : mozilla::ThreadSafeAutoRefCnt
   +0x020 mScheme          : nsTString<char>
   +0x030 mPath            : nsTString<char>
   +0x040 mRef             : nsTString<char>
   +0x050 mQuery           : nsTString<char>
   +0x060 mIsRefValid      : Bool
   +0x061 mIsQueryValid    : Bool

And this is the layout in 137 (still x64):

dt xul!mozilla::net::nsSimpleURI
   +0x018 mRefCnt          : mozilla::ThreadSafeAutoRefCnt
   +0x020 mSpec            : nsTString<char>
   +0x030 mPathSep         : Int4B
   +0x034 mQuerySep        : Int4B
   +0x038 mRefSep          : Int4B

The problem is that chromesafe64.dll relies on the layout of these Mozilla-internal objects. In the crash I observed, they intend to read the string behind mPath, but when they think that they are collecting mPath.mData they are actually reading 4 bytes from mPathSep and 4 bytes from mQuerySep. These values are set by us here in nsSimpleURI::SetSpecInternal in my case, where they were respectively 4 (the offset of the first : in blob:resource://pdf.js/8cf...) and -1 (kNotFound). Firefox crashed because chromesafe64.dll derefenced ffffffff00000004 as a pointer, thinking that there was a path to be read behind this pointer (which is not a pointer anymore as of version 137).

The corresponding code path in chromesafe64.dll 2.0.0.40 is located at RVA 0x2f0f8. It already has what looks like Firefox-version-dependent code there, because I can see a condition that decides if we will look for the path at 0x30 or at 0x20 (which I presume was where mPath was located in an older version of Firefox).

Comment 8

[Yannis Juglaret [:yannis]]

I confirmed locally that blocking chromesafe64.dll through about:third-party makes the crash go away and restores the normal behavior when following the steps from comment 5.

Comment 9

[Greg Stoll :gstoll]

Attachment: Bug 1958112 - block Qihoo chromesafe64.dll versions to avoid crashes r=yjuglaret!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbd055d2a9c5
block Qihoo chromesafe64.dll versions to avoid crashes r=yjuglaret,win-reviewers

Comment 11

[Greg Stoll :gstoll]

Attachment: Bug 1958112 - block Qihoo chromesafe64.dll versions to avoid crashes r=yjuglaret!

Original Revision: https://phabricator.services.mozilla.com/D244481

Comment 12

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: frequent crashes with Qihoo 360 Internet Security
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: See STR on the bug
• Risk associated with taking this patch: low
• Explanation of risk level: Just blocking a DLL, and Yannis has verified blocking this DLL via about:third-party fixes the crash and doesn't hit any other bad behavior
• String changes made/needed: no
• Is Android affected?: no

Comment 13

[Greg Stoll :gstoll]

Attachment: Bug 1958112 - block Qihoo chromesafe64.dll versions to avoid crashes r=yjuglaret!

Original Revision: https://phabricator.services.mozilla.com/D244481

Comment 14

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: frequent crashes with Qihoo 360 Internet Security
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: See STR on the bug
• Risk associated with taking this patch: low
• Explanation of risk level: Just blocking a DLL, and Yannis has verified blocking this DLL via about:third-party fixes the crash and doesn't hit any other bad behavior
• String changes made/needed: no
• Is Android affected?: no

Comment 15

[Yannis Juglaret [:yannis]]

I confirmed that the 32-bit version of Firefox 137 is not affected by this crash. We have received no x86 crash, and the STR from comment 5 do not yield a crash despite the injection of chromesafe.dll (which is the 32-bit variant of chromesafe64.dll).

Comment 16

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/fbd055d2a9c5

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/29ba15d8fe69

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/8be53f705047

Comment 19

[Bogdan Maris, Desktop Test Engineering]

Reproduced the crash with the steps from comment 5 using 137.0 (https://crash-stats.mozilla.org/report/index/069dead9-c0ac-4ee8-9bed-b3e500250408). Verified that using Firefox Release 137.0.1, Firefox Beta 138.0b4 and latest Nightly 139.0a1 on Windows 10 (dedicated machine) the crash is no longer reproducible and the chromesafe64.dll is no longer listed in about:third-party.