Here's the hazard: Error: External function Location: _ZN13nsCanvasFrame24AppendAnonymousContentToER8nsTArrayIP10nsIContentEj$void nsCanvasFrame::AppendAnonymousContentTo(nsTArray<nsIContent*>*, uint32) ### SafeArguments: <arg0> <arg1> Stack Trace: _ZL38AppendNativeAnonymousChildrenFromFrameP8nsIFrameR8nsTArrayIP10nsIContentEj$nsContentUtils.cpp:void AppendNativeAnonymousChildrenFromFrame(nsIFrame*, class nsTArray<nsIContent*>*, uint32) @ dom/base/nsContentUtils.cpp#10528 ### SafeArguments: aKids aFlags _ZN14nsContentUtils29AppendNativeAnonymousChildrenEPK10nsIContentR8nsTArrayIPS0_Ej$void nsContentUtils::AppendNativeAnonymousChildren(nsIContent*, class nsTArray<nsIContent*>*, uint32) @ dom/base/nsContentUtils.cpp#10548 ### SafeArguments: aKids aFlags Gecko_GetAnonymousContentForElement @ layout/style/ServoBindings.cpp#177 The problem is that nsCanvasFrame::AppendAnonymousContentTo(nsTArray<nsIContent*>*, uint32) is incorrectly considered to be unknown, because the callgraph only shows nsCanvasFrame::AppendAnonymousContentTo(class nsTArray<nsIContent*>*, uint32) (notice the "class " prefix.) I think this will probably be in sixgill, and I even have a memory of changing something related. (It's possible the reporting error led me to falsely thinking that my change was ok.)

This is happening for virtual function resolution. The JS code iterates over all function members of relevant types, each of which is associated with a stringified type. It's that type that is missing the "class ". These are stored directly in src_comp.xdb, so it does appear to be a sixgill issue.

Ugh. So I fixed this, but it requires updating sixgill, which is a toolchain job that has to pull down gcc and some things it depends on, which includes mpc-0.8.2, which has a signature that our build script checks, and that signature seems to only exist on the original download server (http://www.multiprecision.org), and that server is down. :-( glandium: https://taskcluster-artifacts.net/eKECsheNRdymvIliGmCPOQ/0/public/logs/live_backing.log shows the failure fetching http://www.multiprecision.org/downloads/mpc-0.8.2.tar.gz It's not hard to find another copy floating around on the web, and some of them even have checksums (from the same server). If I happened to already have a known-good copy, I would hack the build script to rely on a checksum committed to the tree instead of the gpg signature. But I don't, and it seems weird to say "the security mechanism here is unavailable so I'll download some random thing from the web and trust it." We probably need to do something about this single point of failure.

Oops, meant to needinfo glandium for the previous comment.

Well, it works, now.

As for the SPoF, we're probably going to cache those upstream files.

This is reporting a bug in sixgill (false positive), or a security bug in Firefox that sixgill should have caught? I'm not sure how to rate/triage this as a hidden security bug.

It's a bug in sixgill, very probably a false positive, but it's one that could be concealing other true positives. I would say sec-audit.

Ok, I could have sworn I submitted a review request for this already. I must have written up the comment but then lost it somehow. This is https://hg.mozilla.org/users/sfink_mozilla.com/sixgill/rev/739c064cd73c Note that I'm planning on landing several other sixgill changes with this one; I would be updating from 59b74c2e21bd to 39b87ac48871 on https://hg.mozilla.org/users/sfink_mozilla.com/sixgill so feel free to look at any of these changes: 0b44324477fa Cross-check the compile-time plugin headers vs the runtime host gcc 4c4b8a423acb Improve lambda check (there can be <lambda(foo)>) f5d7d11116db File-qualify anonymous structs 90be728e1624 Hack for lambdas that return their own type aeabb6e70a8c minor unsigned comparison fix 5e83950f12de Handle the case where DECL_CONTEXT(returnval) is null 739c064cd73c Pass same flags to decl_as_string for unnamed CSUs as used for all other _as_string calls 935a5a61a15c wrong header for VOID_CST check, at least for newer gcc 3fc74e1b5b05 Refactor debug logging for basecc 635449e1c280 Handle a few more compile flags Want to set up a github repo for this?

I guess I'll ask for blanket review of all of those changes with this. Note that something in here is a prerequisite for gcc6 as well.

Comment on attachment 8958312 [details] [diff] [review] sixgill-changes.patch Review of attachment 8958312 [details] [diff] [review]: ----------------------------------------------------------------- This looks like the wrong patch.

(In reply to Brian Hackett (:bhackett) from comment #10) > > This looks like the wrong patch. It's the right patch, it just doesn't have any of the relevant code. See comment 8 for links to see the actual code. (It's a whole series of patches; I'm not sure which ones you'd find worth reviewing.) But I'm inclined to push this and address review comments later. I know. I'll push this, but create a non-security bug for the rest of the sixgill changes and attach the patches directly there.

Forked off bug 1445553.

https://hg.mozilla.org/integration/mozilla-inbound/rev/26dacf2a27a7566534d9c312b72cb9d1202db755 https://hg.mozilla.org/mozilla-central/rev/26dacf2a27a7