Closed Bug 1538810 Opened 6 years ago Closed 6 years ago

Third-party deps under Google-Breakpad in m-c are outdated

Categories

(Toolkit :: Crash Reporting, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: mhoye, Assigned: gsvelto)

References

(Blocks 1 open bug)

Details

(Keywords: sec-audit, Whiteboard: [adv-main68-])

Attachments

(2 files)

Under: toolkit/crashreporter/google-breakpad/src/third_party:

  • Most importantly: the version of musl-libc we have in-tree is several versions and at least one CVE (CVE-2016-8859) behind of mainstream current (1.1.14 in-tree, 1.1.22 is current)

Some other stuff:

  • Curl is headers only, but they're extremely out of date. Libdisasm is also out of date but afaict has been long abandoned upstream.

  • Libdisasm is also outdated, but looks like it's long-abandoned upstream.

  • Linux\include\gflags dates to 2009

    • there's a duplicate ver. in-tree at media/webrtc/trunk/third_party
Flags: needinfo?(gsvelto)
Keywords: sec-audit

I don't think we're using libdisasm, at least not in Firefox but I have to check. Updating curl and musl headers shouldn't be a problem. I'll leave the NI until I have time to double-check.

So, I'm cooking up a patch to update both curl and musl headers plus the rest of breakpad to the current upstream. As for gflags I'm not sure why it's there; it's not used in our sources nor in upstream breakpad so I can remove it. Last but not least libdisasm is being used in the minidump-analyzer, a tool that we run on the user machine to gather stack traces after Firefox has crashed, and only on Windows. I can disable it if it poses a security risk since we don't use the data that it generates.

Flags: needinfo?(gsvelto) → needinfo?(dveditz)
Flags: needinfo?(dveditz)

This includes improvements to the Linux exception handler that will provide
crash addresse for a number of signals by reading the NT_SIGINFO structure and
quites warnings in a number of files. This also removes an unused header.

I've cooked up a patch to update the curl & musl headers and delete the unused glfags include. We're still using libdisasm in the code but it's not critical. It's not used in Gecko, just in the minidump-analyzer we run to scan the minidump after Firefox crashes. If it's not a liability I'd keep it around because it might be useful for other bugs (e.g. bug 1493342).

Flags: needinfo?(mhoye)

My understanding is that libdisasm doesn't support the x64 instruction set (or any x86 extensions after SSE3) and never will; I don't know what that implies, so somebody better-informed than me will need to evaluate the present and long-term utility of having it around. The authors of libdisasm created OpDis - a wrapper around GNU Binutils - as a replacement:

http://mkfs.github.io/content/opdis/

That sure doesn't look like a drop-in replacement, though; that looks like a side project. So, I don't know what the right call here is. Leaving it in doesn't set us back, that I can see?

Flags: needinfo?(mhoye)

(In reply to Mike Hoye [:mhoye] from comment #6)

My understanding is that libdisasm doesn't support the x64 instruction set (or any x86 extensions after SSE3) and never will; I don't know what that implies, so somebody better-informed than me will need to evaluate the present and long-term utility of having it around.

If it doesn't support x64 then it's not much use and we can just disable it. We're planning to rewrite some of this stuff anyway and some limited disassembly information will be required for things like bug 1493342 so we'll probably implement it ourself (or find an appropriate Rust crate, I'm sure there must be one around already).

The authors of libdisasm created OpDis - a wrapper around GNU Binutils - as a replacement:

http://mkfs.github.io/content/opdis/

That sure doesn't look like a drop-in replacement, though; that looks like a side project. So, I don't know what the right call here is. Leaving it in doesn't set us back, that I can see?

If I disable it the only thing that will be removed is the exploitability field in crash pings, but if it's only ever been populated in 32-bit builds then it's not much use anyway.

Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Summary: Third-party deps under Google-Breakpad in m-c are outdated, possibly vulnerable. → Third-party deps under Google-Breakpad in m-c are outdated
Group: firefox-core-security
Type: enhancement → task

Socorro exposes the exploitability data from the minidump in search and also in an exploitability report. I don't know if it's used, but we do have at least one bug to improve it.

Blocks: 1543910

I filed bug 1543910 for the libdisasm work, in the meantime I'll land the updates here.

Pushed by gsvelto@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/98bf8629efc6 Update breakpad to upstream revision 4d550cceca107f36c4bc1ea1126b7d32cc50f424 r=froydnj https://hg.mozilla.org/integration/autoland/rev/5e05dde4f7cb Updated breakpad's curl and musl headers r=froydnj
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Whiteboard: [adv-main68-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: