Re: [PATCH] Vtable pointer verification, gcc changes (patch 2 of 2)

[Xinliang David Li <davidxl@google.com>]

See some random comments below.  Some test cases should also be added.
It should be easy to fake the attack by using placement new with
incompatible type ..

David


>  /* Start the process of running a particular set of global constructors
>     or destructors.  Subroutine of do_[cd]tors.  */
>
> -static tree
> -start_objects (int method_type, int initp)
> +tree
> +start_objects (int method_type, int initp, const char *extra_name)


Why do you need to make this global? The name start_objects are too
short and can
cause name conflicts. If you really need it to be global, defining a
wrapper function
with longer name as a global seems better.

The new parameter is not documented.


>  {
>    tree body;
>    tree fndecl;
> -  char type[14];
> +  char *type = NULL;
>
>    /* Make ctor or dtor function.  METHOD_TYPE may be 'I' or 'D'.  */
>
> @@ -2984,15 +2982,22 @@ start_objects (int method_type, int init
>        joiner = '_';
>  #endif
>
> -      sprintf (type, "sub_%c%c%.5u", method_type, joiner, initp);
> +      type = (char *) xmalloc ((17 + strlen (extra_name)) * sizeof (char));
> +      sprintf (type, "sub_%c%c%.5u%s", method_type, joiner, initp, extra_name);


Why changing the type name?


>      }
>    else
> -    sprintf (type, "sub_%c", method_type);
> +    {
> +      type = (char *) xmalloc (5 * sizeof (char));
> +      sprintf (type, "sub_%c", method_type);
> +    }
>
>    fndecl = build_lang_decl (FUNCTION_DECL,
>      get_file_function_name (type),
>      build_function_type_list (void_type_node,
>        NULL_TREE));
> +
> +  free (type);
> +
>    start_preparsed_function (fndecl, /*attrs=*/NULL_TREE, SF_PRE_PARSED);
>
>    TREE_PUBLIC (current_function_decl) = 0;
> @@ -3018,7 +3023,7 @@ start_objects (int method_type, int init
>  /* Finish the process of running a particular set of global constructors
>     or destructors.  Subroutine of do_[cd]tors.  */
>
> -static void
> +tree

Document the return value.


>  finish_objects (int method_type, int initp, tree body)
>  {
>    tree fn;
> @@ -3031,6 +3036,10 @@ finish_objects (int method_type, int ini
>      {
>        DECL_STATIC_CONSTRUCTOR (fn) = 1;
>        decl_init_priority_insert (fn, initp);
> +
> +      if (flag_vtable_verify
> +          && strstr (IDENTIFIER_POINTER (DECL_NAME (fn)), ".vtable"))
> +        return fn;
>      }
>    else
>      {
> @@ -3039,6 +3048,7 @@ finish_objects (int method_type, int ini
>      }
>
>
> ===================================================================
> --- gcc/cp/vtable-class-hierarchy.c (revision 0)
> +++ gcc/cp/vtable-class-hierarchy.c (revision 0)
> @@ -0,0 +1,918 @@

Please add documentation (comments) for all functions defined in this file.

Some high level description of implementation structure at
the top of the file may also be helpful.

>
> --- gcc/tree-vtable-verify.c (revision 0)
> +++ gcc/tree-vtable-verify.c (revision 0)

Same comments as above.

On Mon, Nov 5, 2012 at 9:48 AM, Caroline Tice <cmtice@google.com> wrote:
> As requested, I have split the original patch into two parts: GCC
> changes and runtime library changes.  The attached patch is fore the
> gcc changes.
>
> -- Caroline Tice
> cmtice@google.com
>
> 2012-11-05  Caroline Tice  <cmtice@google.com>
>
>         * tree.h (save_vtable_map_decl): New function decl.
>         * tree-pass.h (pass_vtable_verify): New pass declaration.
>         * cp/init.c (build_vtbl_address): Remove 'static' qualifier from
>         function declaration and definition.
>         * cp/class.c (finish_struct_1):  Add call to vtv_save_class_info,
>         if the vtable verify flag is set.
>         * cp/Make-lang.in: Add vtable-class-hierarchy.o to list of object
>         files.  Add definition for building vtable-class-hierarchy.o.
>         * cp/pt.c (mark_class_instantiated):  Add call to vtv_save_class_info
>         if the vtable verify flag is set.
>         * cp/decl2 (start_objects): Remove 'static' qualifier from function
>         declaratin and definition.  Add new paramater, 'extra_name'.  Change
>         'type' var from char array to char *.  Call xmalloc & free for 'type'.
>         Add 'extra_name' to 'type' string.
>         (finish_objects): Remove 'static' qualifier from function declaration
>         and definition. Change return type from void to tree.  Make function
>         return early if we're doing vtable verification and the function is
>         a vtable verification constructor init function.  Make this function
>         return 'fn'.
>         (generate_ctor_or_dtor_function):  Add third argument to calls to
>         start_objects.
>         (cp_write_global_declarations):  Add calls to vtv_recover_class_info,
>         vtv_compute_class_hierarchy_transitive_closure, and
>         vtv_generate_init_routine, if the vtable verify flag is set.
>         * cp/config-lang.in (gtfiles): Add vtable-class-hierarchy.c to the
>         list of gtfiles.
>         * cp/vtable-class-hierarchy.c: New file.
>         * cp/mangle.c (get_mangled_id): Remove static qualifier from function
>         definition.
>         * cp/cp-tree.h:  Add extern function declarations for start_objects,
>         finish_objects, build_vtbl_address, get_mangled_id,
>         vtv_compute_class_hierarchy_transitive_closure,
>         vtv_generate_init_routine, vtv_save_class_info and
>         vtv_recover_class_info.
>         * timevar.def: Add TV_VTABLE_VERIFICATION.
>         * flag-types.h: Add enum vtv_priority defintion.
>         * tree-vtable-verify.c: New file.
>         * tree-vtable-verify.h: New file.
>         * common.opt:  Add definitions for fvtable-verify= and its string
>         options (vtv_priority enum values).
>         * varasm.c (assemble_variable):  Check to see if the variable is a
>         vtable map variable, and if so, put it into the vtable map variable
>         section, and make it comdat.
>         (assemble_vtv_preinit_initializer): New function, to put the
>         vtable verification constructor initialization function in the preinit
>         array, if appropriate.
>         * output.h: Add extern declaration for
>         assemble_vtv_preinit_initializer.
>         * Makefile.in: Add tree-vtable-verify.o to list of OBJS.  Add build
>         rule for tree-vtable-verify.o Add tre-vtable-verify.c to list of source
>         files.
>         * passes.c (init_optimization_passes): Add pass_vtable_verify.
>
> On Thu, Nov 1, 2012 at 1:07 PM, Caroline Tice <cmtice@google.com> wrote:
>> We have been developing a new security hardening feature for GCC that
>> is designed to detect and handle (during program execution) when a
>> vtable pointer that is about to be used for a virtual function call is
>> not a valid vtable pointer for that call (i.e. it has become
>> corrupted, possibly due to a  hacker attack).  We gave a presentation
>> on this work at the Gnu Tools Cauldron in Prague last July.  We now
>> have the implementation fully working and are submitting this patch
>> for review.  We would like to get this into the next release of GCC if
>> possible.
>>
>> The general idea is to collect class hierarchy and vtable pointer data
>> while parsing the classes, then use this data to generate (at runtime)
>> sets of valid vtable pointers, one for each class.  We also find every
>> virtual function call and insert a verification call before the
>> virtual function call.  The verification call takes the set of valid
>> vtable pointers for the declared class of the object, and the actual
>> vtable pointer in the object.  If the vtable pointer in the object is
>> in the set of valid vtable pointers for the object, then verification
>> succeeds and the virtual call is allowed.  Otherwise verification
>> fails and the program aborts.
>>
>> We have a written a more detailed design document, which I am also
>> attaching to this email (GCCVtableSecurityHardeningProposal.txt).
>>
>> The implementation can be divided into roughly two parts:
>> modifications to the main gcc compiler, for things that happen at
>> compile time (collecting the class hierarchy & vtable information;
>> generating the runtime calls to build the data sets from this data;
>> inserting calls to the verification function); and modifications to
>> the runtime, i.e. functions that go into libstdc++ for building the
>> data sets, for doing the verification against the data sets, for
>> protecting the memory where the data sets reside, etc.).
>>
>> Please let me know if there is any more information you need, or if
>> you have any questions about this patch.
>>
>> -- Caroline Tice
>> cmtice@google.com
>>
>> libstdc++/ChangeLog
>>
>> 2012-11-01  Caroline Tice  <cmtice@google.com>
>>
>>         * src/Makefile.am: Add libvtv___la_LIBDD definition; update CXXLINK
>>         to search in libvtv___la_LIBADD and to link in libvtv_init.
>>         * src/Makefile.in: Regenerate.
>>         * libsupc++/Makefile.am: Add libvtv_init.la and libvtv_stubs.la to
>>         toolexeclib_LTLIBRARIES.  Add vtv_rts.cc, vtv_malloc.cc and
>>         vtv_utils.cc to sources.  Define vtv_init_sources and
>>         vtv_stubs_sources.  Also define libvtv_init_la_SOURCES and
>>         libvtv_stubs_la_sources.
>>         * libsupc++/Makefile.in: Regenerate.
>>         * libsupc++/vtv_rts.cc: New file.
>>         * libsupc++/vtv_malloc.h: New file.
>>         * libsupc++/vtv_rts.h: New file.
>>         * libsupc++/vtv_fail.h: New file.
>>         * libsupc++/vtv_set.h: New file.
>>         * libsupc++/vtv_stubs.cc: New file.
>>         * libsupc++/vtv_utils.cc: New file.
>>         * libcupc++/vtv_utils.h: New file.
>>         * libsupc++/vtv_init.cc: New file.
>>         * libsupc++/vtv_malloc.cc: New file.
>>         * config/abi/pre/gnu.ver (GLIBCXX_3.4.18): Add vtable verification
>>         functions and vtable map variables to library export list.
>>
>> gcc/ChangeLog:
>>
>> 2012-11-01  Caroline Tice  <cmtice@google.com>
>>
>>         * tree.h (save_vtable_map_decl): New function decl.
>>         * tree-pass.h (pass_vtable_verify): New pass declaration.
>>         * cp/init.c (build_vtbl_address): Remove 'static' qualifier from
>>         function declaration and definition.
>>         * cp/class.c (finish_struct_1):  Add call to vtv_save_class_info,
>>         if the vtable verify flag is set.
>>         * cp/Make-lang.in: Add vtable-class-hierarchy.o to list of object
>>         files.  Add definition for building vtable-class-hierarchy.o.
>>         * cp/pt.c (mark_class_instantiated):  Add call to vtv_save_class_info
>>         if the vtable verify flag is set.
>>         * cp/decl2 (start_objects): Remove 'static' qualifier from function
>>         declaratin and definition.  Add new paramater, 'extra_name'.  Change
>>         'type' var from char array to char *.  Call xmalloc & free for 'type'.
>>         Add 'extra_name' to 'type' string.
>>         (finish_objects): Remove 'static' qualifier from function declaration
>>         and definition. Change return type from void to tree.  Make function
>>         return early if we're doing vtable verification and the function is
>>         a vtable verification constructor init function.  Make this function
>>         return 'fn'.
>>         (generate_ctor_or_dtor_function):  Add third argument to calls to
>>         start_objects.
>>         (cp_write_global_declarations):  Add calls to vtv_recover_class_info,
>>         vtv_compute_class_hierarchy_transitive_closure, and
>>         vtv_generate_init_routine, if the vtable verify flag is set.
>>         * cp/config-lang.in (gtfiles): Add vtable-class-hierarchy.c to the
>>         list of gtfiles.
>>         * cp/vtable-class-hierarchy.c: New file.
>>         * cp/mangle.c (get_mangled_id): Remove static qualifier from function
>>         definition.
>>         * cp/cp-tree.h:  Add extern function declarations for start_objects,
>>         finish_objects, build_vtbl_address, get_mangled_id,
>>         vtv_compute_class_hierarchy_transitive_closure,
>>         vtv_generate_init_routine, vtv_save_class_info and
>>         vtv_recover_class_info.
>>         * timevar.def: Add TV_VTABLE_VERIFICATION.
>>         * flag-types.h: Add enum vtv_priority defintion.
>>         * tree-vtable-verify.c: New file.
>>         * tree-vtable-verify.h: New file.
>>         * common.opt:  Add definitions for fvtable-verify= and its string
>>         options (vtv_priority enum values).
>>         * varasm.c (assemble_variable):  Check to see if the variable is a
>>         vtable map variable, and if so, put it into the vtable map variable
>>         section, and make it comdat.
>>         (assemble_vtv_preinit_initializer): New function, to put the
>>         vtable verification constructor initialization function in the preinit
>>         array, if appropriate.
>>         * output.h: Add extern declaration for
>>         assemble_vtv_preinit_initializer.
>>         * Makefile.in: Add tree-vtable-verify.o to list of OBJS.  Add build
>>         rule for tree-vtable-verify.o Add tre-vtable-verify.c to list of source
>>         files.
>>         * passes.c (init_optimization_passes): Add pass_vtable_verify.