Re: RFA/RFC: Add stack recursion limit to libiberty's demangler [v4]

[Nick Clifton <nickc at redhat dot com>]

Hi Ian,

>>> Shouldn't we make it fool-proof by instead introducing a DMGL_NO_RECURSION_LIMIT

> You don't need my blessing--I wrote that code ages ago--but I agree
> with Richard that in practice it's OK to limit recursion depth by
> default.  Real symbols have very limited recursion requirements.

OK then, here is a fourth revision of the patch.

In this version:

  * The demangler option has been renamed to DMGHL_NO_RECURSE_LIMIT
    and if the option is not present then the limit is enforced.

  * I also found another PR that is fixed by the patch, although I had
    to make sure that the affected code could handle NULL pointers 
    properly afterwards.

  OK to apply ?

Cheers
  Nick


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

	* demangle.h (DMGL_NO_RECURSE_LIMIT): Define.
        (DEMANGLE_RECURSION_LIMIT): Define

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

	PR 87681
	PR 87675
	PR 87636
	PR 87350
	PR 87335
	* cp-demangle.h (struct d_info): Add recursion_level field.
	* cp-demangle.c (d_function_type): Add recursion counter.
	If the recursion limit is reached and the check is not disabled,
	then return with a failure result.
	(cplus_demangle_init_info): Initialise the recursion_level field.
        (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-dem.c (struct work): Add recursion_level field.
	(squangle_mop_up): Set the numb and numk fields to zero.
	(work_stuff_copy_to_from): Handle the case where a btypevec or 
	ktypevec field is NULL.
	(demangle_nested_args): Add recursion counter.  If
	the recursion limit is not disabled and reached, return with a
	failure result.

Index: include/demangle.h
===================================================================
--- include/demangle.h	(revision 266771)
+++ include/demangle.h	(working copy)
@@ -68,6 +68,17 @@
 /* If none of these are set, use 'current_demangling_style' as the default. */
 #define DMGL_STYLE_MASK (DMGL_AUTO|DMGL_GNU|DMGL_LUCID|DMGL_ARM|DMGL_HP|DMGL_EDG|DMGL_GNU_V3|DMGL_JAVA|DMGL_GNAT|DMGL_DLANG|DMGL_RUST)
 
+/* Disable a limit on the depth of recursion in mangled strings.
+   Note if this limit is disabled then stack exhaustion is possible when
+   demangling pathologically complicated strings.  Bug reports about stack
+   exhaustion when the option is enabled will be rejected.  */  
+#define DMGL_NO_RECURSE_LIMIT (1 << 18)	
+
+/* If DMGL_NO_RECURE_LIMIT is not enabled, then this is the value used as
+   the maximum depth of recursion allowed.  It should be enough for any
+   real-world mangled name.  */
+#define DEMANGLE_RECURSION_LIMIT 1024
+  
 /* Enumeration of possible demangling styles.
 
    Lucid and ARM styles are still kept logically distinct, even though
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c	(revision 266771)
+++ libiberty/cp-demangle.c	(working copy)
@@ -2852,21 +2852,35 @@
 static struct demangle_component *
 d_function_type (struct d_info *di)
 {
-  struct demangle_component *ret;
+  struct demangle_component *ret = NULL;
 
-  if (! d_check_char (di, 'F'))
-    return NULL;
-  if (d_peek_char (di) == 'Y')
+  if ((di->options & DMGL_NO_RECURSE_LIMIT) == 0)
     {
-      /* Function has C linkage.  We don't print this information.
-	 FIXME: We should print it in verbose mode.  */
-      d_advance (di, 1);
+      if (di->recursion_level > DEMANGLE_RECURSION_LIMIT)
+	/* FIXME: There ought to be a way to report
+	   that the recursion limit has been reached.  */
+	return NULL;
+
+      di->recursion_level ++;
     }
-  ret = d_bare_function_type (di, 1);
-  ret = d_ref_qualifier (di, ret);
 
-  if (! d_check_char (di, 'E'))
-    return NULL;
+  if (d_check_char (di, 'F'))
+    {
+      if (d_peek_char (di) == 'Y')
+	{
+	  /* Function has C linkage.  We don't print this information.
+	     FIXME: We should print it in verbose mode.  */
+	  d_advance (di, 1);
+	}
+      ret = d_bare_function_type (di, 1);
+      ret = d_ref_qualifier (di, ret);
+      
+      if (! d_check_char (di, 'E'))
+	ret = NULL;
+    }
+
+  if ((di->options & DMGL_NO_RECURSE_LIMIT) == 0)
+    di->recursion_level --;
   return ret;
 }
 
@@ -6203,6 +6217,7 @@
   di->expansion = 0;
   di->is_expression = 0;
   di->is_conversion = 0;
+  di->recursion_level = 0;
 }
 
 /* Internal implementation for the demangler.  If MANGLED is a g++ v3 ABI
@@ -6242,6 +6257,20 @@
 
   cplus_demangle_init_info (mangled, options, strlen (mangled), &di);
 
+  /* PR 87675 - Check for a mangled string that is so long
+     that we do not have enough stack space to demangle it.  */
+  if (((options & DMGL_NO_RECURSE_LIMIT) == 0)
+      /* This check is a bit arbitrary, since what we really want to do is to
+	 compare the sizes of the di.comps and di.subs arrays against the
+	 amount of stack space remaining.  But there is no portable way to do
+	 this, so instead we use the recursion limit as a guide to the maximum
+	 size of the arrays.  */
+      && (unsigned long) di.num_comps > DEMANGLE_RECURSION_LIMIT)
+    {
+      /* FIXME: We need a way to indicate that a stack limit has been reached.  */
+      return 0;
+    }
+
   {
 #ifdef CP_DYNAMIC_ARRAYS
     __extension__ struct demangle_component comps[di.num_comps];
Index: libiberty/cp-demangle.h
===================================================================
--- libiberty/cp-demangle.h	(revision 266771)
+++ libiberty/cp-demangle.h	(working copy)
@@ -122,6 +122,9 @@
   /* Non-zero if we are parsing the type operand of a conversion
      operator, but not when in an expression.  */
   int is_conversion;
+  /* If DMGL_NO_RECURSE_LIMIT is not active then this is set to
+     the current recursion level.  */
+  unsigned int recursion_level;
 };
 
 /* To avoid running past the ending '\0', don't:
Index: libiberty/cplus-dem.c
===================================================================
--- libiberty/cplus-dem.c	(revision 266771)
+++ libiberty/cplus-dem.c	(working copy)
@@ -146,6 +146,7 @@
   int *proctypevec;     /* Indices of currently processed remembered typevecs.  */
   int proctypevec_size;
   int nproctypes;
+  unsigned int recursion_level;
 };
 
 #define PRINT_ANSI_QUALIFIERS (work -> options & DMGL_ANSI)
@@ -1292,6 +1293,7 @@
       free ((char *) work -> btypevec);
       work->btypevec = NULL;
       work->bsize = 0;
+      work->numb = 0;
     }
   if (work -> ktypevec != NULL)
     {
@@ -1298,6 +1300,7 @@
       free ((char *) work -> ktypevec);
       work->ktypevec = NULL;
       work->ksize = 0;
+      work->numk = 0;
     }
 }
 
@@ -1331,8 +1334,15 @@
 
   for (i = 0; i < from->numk; i++)
     {
-      int len = strlen (from->ktypevec[i]) + 1;
+      int len;
 
+      if (from->ktypevec[i] == NULL)
+	{
+	  to->ktypevec[i] = NULL;
+	  continue;
+	}
+
+      len = strlen (from->ktypevec[i]) + 1;
       to->ktypevec[i] = XNEWVEC (char, len);
       memcpy (to->ktypevec[i], from->ktypevec[i], len);
     }
@@ -1342,8 +1352,15 @@
 
   for (i = 0; i < from->numb; i++)
     {
-      int len = strlen (from->btypevec[i]) + 1;
+      int len;
 
+      if (from->btypevec[i] == NULL)
+	{
+	  to->btypevec[i] = NULL;
+	  continue;
+	}
+
+      len = strlen (from->btypevec[i]) + 1;
       to->btypevec[i] = XNEWVEC (char , len);
       memcpy (to->btypevec[i], from->btypevec[i], len);
     }
@@ -1401,6 +1418,7 @@
 
       free ((char*) work->tmpl_argvec);
       work->tmpl_argvec = NULL;
+      work->ntmpl_args = 0;
     }
   if (work->previous_argument)
     {
@@ -4478,6 +4496,7 @@
 }
 
 /* Lose all the info related to B and K type codes. */
+
 static void
 forget_B_and_K_types (struct work_stuff *work)
 {
@@ -4503,6 +4522,7 @@
 	}
     }
 }
+
 /* Forget the remembered types, but not the type vector itself.  */
 
 static void
@@ -4693,10 +4713,21 @@
 demangle_nested_args (struct work_stuff *work, const char **mangled,
                       string *declp)
 {
+  static unsigned long recursion_level = 0;
   string* saved_previous_argument;
   int result;
   int saved_nrepeats;
 
+  if ((work->options & DMGL_NO_RECURSE_LIMIT) == 0
+      && work->recursion_level > DEMANGLE_RECURSION_LIMIT)
+    {
+      /* FIXME: There ought to be a way to report that the recursion limit
+	 has been reached.  */
+      return 0;
+    }
+
+  recursion_level ++;
+
   /* The G++ name-mangling algorithm does not remember types on nested
      argument lists, unless -fsquangling is used, and in that case the
      type vector updated by remember_type is not used.  So, we turn
@@ -4723,6 +4754,7 @@
   --work->forgetting_types;
   work->nrepeats = saved_nrepeats;
 
+  --recursion_level;
   return result;
 }