1696579 - Authenticode Signatures are broken in crash-stats

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

Suppose an injected DLL modifies xul.dll in some way such that xul.dll later crashes.

Because of the changes made in bug 1649443, the resulting crash report will not contain information such as the injected DLL's authenticode signature, since the DLL was not actuall present on the call stack.

As an example, the modules tab in this crash report only includes the signature for xul.dll.

Comment 1

[Gabriele Svelto [:gsvelto]]

Bug 1649443 only changed the list of modules that are included in crash pings, it did not change how we handle crash reports sent to crash-stats. I don't know how the "Signed By" field is populated but my guess is that this comes from the minidump_stackwalk tool so I'll check that out next week and see why it's not showing signatures for those DLLs.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

This is a regression caused by that bug. The signatures are pulled in minidump-stackwalk by walking that list of modules that is now being filtered by bug 1649443.

Comment 3

[Gabriele Svelto [:gsvelto]]

That's another stack walker. bug 1649443 changed the minidump-analyzer tool that we ship with Firefox and run on the client to generate the stack trace for the crash ping. What we use on Socorro is minidump-stackwalk and it's built from these sources. For added confusion what gets installed on your machine by the bootstrap step is called minidump_stackwalk and it's built from these sources in mozilla-central.

This mess is an historical artifact and we have bug 1487410 on file to try and replace all different stack walkers with just one. In my defense I must say that we used to have a fourth stack walker for automation but now we're down to "just" three.

Comment 4

[Gabriele Svelto [:gsvelto]]

Oh wait, now I get it, we don't extract the signatures on Socorro, we extract them locally and we do it in minidump-analyzer. I'll pick this up to fix it.

Comment 5

[Gabriele Svelto [:gsvelto]]

While writing the fix for this bug and for bug 1702043 I realized we could also include the signatures for the unloaded modules. WDYT? Would that be useful?

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Yeah, if we're including unloaded modules in the report, let's pull their signatures too!

Comment 7

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1696579 - Store all authenticode signaures when analyzing a minidump r=KrisWright

This fixes the regression introduced with bug 1649443 and also includes
signatures from unloaded modules.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da760935080d
Store all authenticode signaures when analyzing a minidump r=KrisWright

Comment 9

[Alexandru Michis [:malexandru]]

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