diff mbox

[0/7] Fix bugs found during demangler fuzz-testing

Message ID 559AD66D.1070809@gmail.com
State New
Headers show

Commit Message

Mikhail Maltsev July 6, 2015, 7:26 p.m. UTC 
Hi, all!
I performed some fuzz-testing of C++ demangler
(libiberty/cp-demangle.c). The test revealed several bugs, which are
addressed by this series of patches.

Here is a brief description of them:
First one adds a macro CHECK_DEMANGLER. When this macro is defined,
d_peek_next_char and d_advance macros are replaced by inline functions
which perform additional sanity checks and call __builtin_abort when
out-of-bound access is detected.
The second patch fixes a syntax error in debug dump code (it is normally
disabled, unless CP_DEMANGLE_DEBUG is defined).
All other parts fix some errors on invalid input. The attached files
contain a cumulative patch and changelog record.
Bootstrapped and regtested on x86_64-linux.

Some notes:
* Patch 4 adds "#include <limits.h>" to demangler (because of INT_MAX).
I noticed that this file is checked by configury scripts. Do we have
hosts, which do not provide this header? If so, what is the appropriate
replacement for it?
* Testcase "_ZDTtl" (fixed in patch 5) did not actually cause segfault,
but it still invoked undefined behavior (read 1 byte past buffer end).
* Testcase "DpDv1_c" (from patch 7) is now demangled as "(char
__vector(1))..." (it used to segfault). I'm not sure, whether it is
correct or should be rejected.

I have some more test cases, lots of them cause infinite recursion,
because of conversion operator being used as template parameter. Some
are fixed in PR61321, some are not. For example, there are cases when
conversion operator is used as a nested (qualified) name:

_Z1fIN1CcvT_EEvv -> segfault
Presumably this means:
template<typename T>
void f<C::operator T>()

I wonder, if it is possible in valid C++ code?

Notice that the following template instantiation is demangled correctly:
void f<C::operator int>()
_Z1fIN1CcviEEvv -> OK
diff mbox

Patch

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 2988b6b..4ca285e 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -103,6 +103,7 @@ 
 #include "config.h"
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 
 #ifdef HAVE_STDLIB_H
@@ -715,7 +716,7 @@  d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_FIXED_TYPE:
       printf ("fixed-point type, accum? %d, sat? %d\n",
               dc->u.s_fixed.accum, dc->u.s_fixed.sat);
-      d_dump (dc->u.s_fixed.length, indent + 2)
+      d_dump (dc->u.s_fixed.length, indent + 2);
       break;
     case DEMANGLE_COMPONENT_ARGLIST:
       printf ("argument list\n");
@@ -1599,7 +1600,7 @@  d_source_name (struct d_info *di)
   struct demangle_component *ret;
 
   len = d_number (di);
-  if (len <= 0)
+  if (len <= 0 || len > INT_MAX)
     return NULL;
   ret = d_identifier (di, len);
   di->last_name = ret;
@@ -3166,6 +3167,8 @@  d_expression_1 (struct d_info *di)
       struct demangle_component *type = NULL;
       if (peek == 't')
 	type = cplus_demangle_type (di);
+      if (!d_peek_next_char (di))
+	return NULL;
       d_advance (di, 2);
       return d_make_comp (di, DEMANGLE_COMPONENT_INITIALIZER_LIST,
 			  type, d_exprlist (di, 'E'));
@@ -3240,6 +3243,8 @@  d_expression_1 (struct d_info *di)
 	    struct demangle_component *left;
 	    struct demangle_component *right;
 
+	    if (code == NULL)
+	      return NULL;
 	    if (op_is_new_cast (op))
 	      left = cplus_demangle_type (di);
 	    else
@@ -3267,7 +3272,9 @@  d_expression_1 (struct d_info *di)
 	    struct demangle_component *second;
 	    struct demangle_component *third;
 
-	    if (!strcmp (code, "qu"))
+	    if (code == NULL)
+	      return NULL;
+	    else if (!strcmp (code, "qu"))
 	      {
 		/* ?: expression.  */
 		first = d_expression_1 (di);
@@ -4196,6 +4203,9 @@  d_find_pack (struct d_print_info *dpi,
     case DEMANGLE_COMPONENT_CHARACTER:
     case DEMANGLE_COMPONENT_FUNCTION_PARAM:
     case DEMANGLE_COMPONENT_UNNAMED_TYPE:
+    case DEMANGLE_COMPONENT_FIXED_TYPE:
+    case DEMANGLE_COMPONENT_DEFAULT_ARG:
+    case DEMANGLE_COMPONENT_NUMBER:
       return NULL;
 
     case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
@@ -4431,6 +4441,11 @@  d_print_comp_inner (struct d_print_info *dpi, int options,
 	    local_name = d_right (typed_name);
 	    if (local_name->type == DEMANGLE_COMPONENT_DEFAULT_ARG)
 	      local_name = local_name->u.s_unary_num.sub;
+	    if (local_name == NULL)
+	      {
+		d_print_error (dpi);
+		return;
+	      }
 	    while (local_name->type == DEMANGLE_COMPONENT_RESTRICT_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_VOLATILE_THIS
 		   || local_name->type == DEMANGLE_COMPONENT_CONST_THIS
diff --git a/libiberty/cp-demangle.h b/libiberty/cp-demangle.h
index 6fce025..c37a91f 100644
--- a/libiberty/cp-demangle.h
+++ b/libiberty/cp-demangle.h
@@ -135,12 +135,41 @@  struct d_info
    - call d_check_char(di, '\0')
    Everything else is safe.  */
 #define d_peek_char(di) (*((di)->n))
-#define d_peek_next_char(di) ((di)->n[1])
-#define d_advance(di, i) ((di)->n += (i))
+#ifndef CHECK_DEMANGLER
+#  define d_peek_next_char(di) ((di)->n[1])
+#  define d_advance(di, i) ((di)->n += (i))
+#endif
 #define d_check_char(di, c) (d_peek_char(di) == c ? ((di)->n++, 1) : 0)
 #define d_next_char(di) (d_peek_char(di) == '\0' ? '\0' : *((di)->n++))
 #define d_str(di) ((di)->n)
 
+/* Define CHECK_DEMANGLER to perform additional sanity checks (i.e., when
+   debugging the demangler).  It will cause some slowdown, but will allow to
+   catch out-of-bound access errors earlier.
+   Note: CHECK_DEMANGLER is not compatible with compilers other that GCC.  */
+#ifdef CHECK_DEMANGLER
+static inline char
+d_peek_next_char (const struct d_info *di)
+{
+  if (!di->n[0])
+    __builtin_abort ();
+  return di->n[1];
+}
+
+static inline void
+d_advance (struct d_info *di, int i)
+{
+  if (i < 0)
+    __builtin_abort ();
+  while (i--)
+    {
+      if (!di->n[0])
+	__builtin_abort ();
+      di->n++;
+    }
+}
+#endif
+
 /* Functions and arrays in cp-demangle.c which are referenced by
    functions in cp-demint.c.  */
 #ifdef IN_GLIBCPP_V3
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 6ea64ae..b58cea2 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4091,6 +4091,36 @@  void g<1>(A<1>&, B<static_cast<bool>(1)>&)
 _ZNKSt7complexIiE4realB5cxx11Ev
 std::complex<int>::real[abi:cxx11]() const
 #
+# Some more crashes revealed by fuzz-testing:
+# Check for NULL pointer when demangling trinary operators
+--format=gnu-v3
+Av32_f
+Av32_f
+# Check range when converting from long to int
+--format=gnu-v3
+_Z11111111111
+_Z11111111111
+# Check out-of-bounds access when decoding braced initializer list
+--format=gnu-v3
+_ZDTtl
+_ZDTtl
+# Check for NULL pointer when demangling DEMANGLE_COMPONENT_LOCAL_NAME
+--format=gnu-v3
+_ZZN1fEEd_lEv
+_ZZN1fEEd_lEv
+# Handle DEMANGLE_COMPONENT_FIXED_TYPE in d_find_pack
+--format=gnu-v3
+DpDFT_
+DpDFT_
+# Likewise, DEMANGLE_COMPONENT_DEFAULT_ARG
+--format=gnu-v3
+DpZ1fEd_
+DpZ1fEd_
+# Likewise, DEMANGLE_COMPONENT_NUMBER (??? result is probably still wrong)
+--format=gnu-v3
+DpDv1_c
+(char __vector(1))...
+#
 # Ada (GNAT) tests.
 #
 # Simple test.