On 1/14/2013 1:05 PM, Jesse Ruderman wrote:
We have a lot of pointer casts in our tree [1][2][3] and some security
holes involve these casts going wrong [4][5].
Should we make debug builds check casts to (vtableful?) pointer types?
This could be done by adding and calling an "assert_cast" function, or
by adding a new "sanitizer" mode [6] to clang.
Non-vtableful types don't carry around identifiers around them to
identify their dynamic type, and, in C code or code of C vintage [1],
knowledge about C struct layout is used to emulate virtual function
dispatch. It's possible to get around this with pointer tagging or
modifying data structures, but the first requires compiler transforms
and probably whole-program analysis [2] and the second would necessitate
rewriting the code.
For types with vtables, these casts are problematic only if we're going
down or sideways in the inheritance tree, so it's only a problem in the
portions of our type tree where we have very deep or broad branches. So,
to me, there's really two places where we have this problem that are
likely to occur:
1. Assuming a specific implementation of an interface--I am aware of
places where we do this in comm-central, for example. This is probably
likely to blow up only when you start involving extensions, so I don't
think testing is beneficial here.
2. Misassuming the dynamic type in our bushy parts of the tree (DOM
elements and the frame tree are the only very-bushy parts I'm aware of
offhand). These type trees are presumably closed, which mean we can give
each class a unique ID for a number and a dynamic cast costs two integer
comparisons, which should be reasonably efficient to do even in
"perf-critical code." If that's too slow, you might be able to arrange
IDs to reduce it to a single bit test. The former approach is what LLVM
uses internally for most of its data structures in lieu of RTTI (see
"dyn_cast" and related methods).
* Is it possible to 'query' vtableful objects by looking at vtable
pointers, like gdb does [7]? Or does this quickly become so
complicated that we might as well enable RTTI and use the same
mechanism as dynamic_cast?
Vtables can only tell you what type an object is, and there is no
inherent information in them that makes class hierarchy discoverable.
That is why you actually gain something by disabling RTTI. You could try
to do more information by walking debug information, but this requires
us to a) parse debug information and b) require it where necessary.
* Should some of these checks be enabled in release builds?
The non-general-purpose implementation I mentioned above strikes me as
sufficiently fast to run in release builds. Any general-purpose
implementation would probably get pushback for performance reasons, but
I tend to be of the persuasion of "do the right thing for security and
let the compiler get rid of it if it's unnecessary."
[1] At least NSPR, NSS, and libffi are C code portions of our tree, and
one part of comm-central (libmime) is effectively C code for the
purposes of this discussion, although I am trying to eliminate that library.
[2] I mean whole-program analysis, not link-time optimization: you need
to be able to uniquely identify all types in the system.
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
