Open Bug 510629 (cfi) Opened 11 years ago Updated 1 month ago

[meta] Ship Control Flow Integrity (CFI)

Categories

(Firefox Build System :: Source Code Analysis, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: brendan, Unassigned)

References

(Depends on 5 open bugs, Blocks 1 open bug, )

Details

(Keywords: meta, parity-chrome, sec-want)

The URL links to a paper that uses points-to and costs 5-10% at runtime. No good for us. But could we do better?

We've discussed by email this idea:

Copy all vtbls to a part of the virtual address space mapped so that we can test quickly (say, "is bit 25 set?") whether a call is going to one of our vtbls, and insert a check -- or just force bit 25 to be set -- in all vitual method calls. We'd have to mangle our constructors to "correct" the vptr(s) in the new instance, or hack into our C++ compilers. We would have to put the XPConnect stub vtbl in the mapped address space.

This is not CFI, because an attacker could still force a method call to go to the wrong method body through the wrong vtbl. But it might be helpful.

First thing to do: analyze the callgraph (bug 507711) and see how many virtual method callsites are monomorphic (noscript method / non-scriptable interface, only one implementation -- ripe for deCOMtamination but we may still have a bunch of these kinds of callsites), and if not, for how many virtual method callsites can we statically bound the polymorphism (ignoring XPConnect).

For XPConnect we can afford to do a CFI check, I claim, since we've quickstubbed the hot DOM paths and are going to JIT-optimize further.

For the monomorphic sites, if any (I bet we have too many still), can we simply deCOMtaminate, that is, devirtualize the callsite? Or even inline if appropriate.

For the polymorphic virtual method callsites, perhaps we can afford to add a full CFI check as developed in the paper at the URL.

Of course the NX bit is important, but as the paper argues, it's not enough. And we have to write code that becomes executable from the JIT. So it seems worth trying for a systematic CFI enforcement mechanism, especially if we can do some enforcement (or just deCOMtamination) statically.

/be
I wonder if we already have incomplete but possibly helpful existing abstraction that could be extended to enforce some kind of CFI: nsCOMPtr. Is there template or other C++ magic we could use with nsCOMPtr::operator-> to check the address of the method being called?

/be
Note that we have monomorphic virtual methods that are virtual just so they can be called across module boundaries...
Those are ripe for optimization too!

Dynamic libraries, another '90s boondoggle :-P.

/be
Depends on: 1302891
Product: Core → Firefox Build System
A note; -fwhole-program-vtables should help with perf.
Okay; I'm going to take this bug over.

The intent of this bug is to capture the process of creating a CFI build and then trying to improve its performance enough until it can be shipped.
Assignee: nobody → tom
Summary: Investigate CFI (Control Flow Integrity) for Mozilla code → [meta] Ship Control Flow Integrity (CFI)
Depends on: 1464193
Keywords: meta
Priority: -- → P3

Interesting that Chrome seems to have put a major effort into this recently.
There is a lot here, on this page and by following links:
https://sites.google.com/a/chromium.org/dev/developers/testing/control-flow-integrity
They use Clang's CFI: https://clang.llvm.org/docs/ControlFlowIntegrity.html

Follow-up: According to the link https://www.chromium.org/developers/testing/control-flow-integrity CFI is now enabled on "the official Chrome on Linux x86-64 (M68 and newer)", has enabled discovery of a large list of bugs (same link), and the direct CPU overhead is negligible, while compiled code size has grown manageably:
"Overhead (only tested on x64)

  • CPU overhead is < 1%
  • Code size overhead is 5%-9%
  • RAM overhead is a small constant (read-only tables inside the binary shared between all chrome processes)"
Assignee: tom → nobody
Type: defect → enhancement
See Also: → cet
You need to log in before you can comment on or make changes to this bug.