Home Original page

RFA/RFC: Add stack recursion limit to libiberty's demangler

Nick Clifton nickc@redhat.com
Thu Nov 29 15:01:00 GMT 2018 
Hi Ian,

  Libiberty's demangler seems to be a favourite target of people looking
  to file new CVEs by fuzzing strings and I have finally gotten tired of
  reviewing them all.  So I would like to propose a patch to add support
  for placing a recursion limit on the demangling functions.

  The patch adds a new demangling option - DMGL_RECURSE_LIMIT - which
  needs to be present in order for the new code to take affect.   So
  current users of the libiberty library will not be affected[*].
  The patch also adds a new function - cplus_demangle_set_recursion_limit()
  - which can be used to probe and/or set the limit.

  When the option is in effect a few of the demangler functions will use
  static counters to make sure that they have not been called
  recursively too many times.  I only chose those functions for which I
  could find filed PRs that triggered this kind of problem.  But with
  the stack limiting framework in place it would be easy to add checks
  to other functions, should they prove necessary.

  I also encountered one binary that could trigger stack exhaustion in
  d_demangle_callback where it tries to allocate stack space to hold
  component and substitution arrays.  So the patch includes a check
  for this situation as well.
  
  There does not appear to be any error reporting framework for the
  demangler functions, so when a recursion limit is reached the
  functions just return a failure/NULL result.

  I have tested the patch with a variety of different binutils builds
  and also bootstrapped an x86_64-pc-linux-gnu toolchain.  No new
  failures were found.

  What do you think, is this approach reasonable ?

Cheers
  Nick

[*] Actually I also have a patch for the binutils to modify the
addr2line, c++filt, nm and objdump programs to make use of this new
feature, should it be accepted into libiberty.

Patches:

include/ChangeLog
2018-11-29  Nick Clifton  <nickc@redhat.com>

	* demangle.h (DMGL_RECURSE_LIMIT): Define.
        (cplus_demangle_set_recursion_limit): Prototype.

libiberty/ChangeLog
2018-11-29  Nick Clifton  <nickc@redhat.com>

	PR 87675
        PR 87636
	* cp-demangle.c (demangle_recursion_limit): New static
        variable.
        (d_function_type): Add recursion counter.  If the recursion
        limit is enabled and reached, return with a failure result.
        (d_demangle_callback): If the recursion limit is enabled, check
	for a mangled string that is so long that there is not enough
	stack space for the local arrays.
        (cplus_demangle_set_recursion): New function.  Sets and/or
	returns the current stack recursion limit.
        * cplus-dem.c (demangle_nested_args): Add recursion counter.  If
	the recursion limit is enabled and reached, return with a
	failure result.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: libiberty-demangler-recursion-limit.patch
Type: text/x-patch
Size: 6036 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20181129/2138c5f4/attachment.bin>
-------------- next part --------------

binutils/ChangeLog
2018-11-29  Nick Clifton  <nickc@redhat.com>

	* addr2line.c (demangle_flags): New static variable.
        (long_options): Add --recurse-limit.
        (translate_address): Pass demangle_flags to bfd_demangle.
        (main): Handle --recuse-limit option.
        * cxxfilt.c (flags): Add DMGL_RECURSE_LIMIT.
        (long_options): Add --recurse-limit.
        (main): Handle --recuse-limit option.
        * dlltool.c (gen_def_file): Include DMGL_RECURSE_LIMIT in flags
        passed to cplus_demangle.
        * nm.c (demangle_flags): New static variable.
        (long_options): Add --recurse-limit.
        (main): Handle --recurse-limit.
        * objdump.c (demangle_flags): New static variable.
        (usage): Add --recurse-limit.
        (long_options): Add --recurse-limit.
        (objdump_print_symname): Pass demangle_flags to bfd_demangle.
        (disassemble_section): Likewise.
        (dump_dymbols): Likewise.
        (main): Handle --recurse-limit.
        * prdbg.c (demangle_flags): New static variable.
        (tg_variable): Pass demangle_flags to demangler.
        (tg_start_function): Likewise.
        * stabs.c (demangle_flags): New static variable.
        (stab_demangle_template): Pass demangle_flags to demangler.
        (stab_demangle_v3_argtypes): Likewise.
        (stab_demangle_v3_arg): Likewise.
	* doc/binutuls.texi: Document new command line options.
	* NEWS: Mention the new feature.
        * testsuite/config/default.exp (CXXFILT): Define if not already
        defined.
        (CXXFILTFLAGS): Likewise.
        * testsuite/binutils-all/cxxfilt.exp: New file.  Runs a few
        simple tests of the cxxfilt program.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: binutils-demangler-recursion-limit.patch
Type: text/x-patch
Size: 23017 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20181129/2138c5f4/attachment-0001.bin>
More information about the Binutils mailing list