RFA: libiberty: Add a limit on demangling qualifiers (PR 87241) (version 2)

RFA: libiberty: Add a limit on demangling qualifiers (PR 87241) (version 2)

[Nick Clifton]

Hi Ian,

  *sigh* 5 minutes after sending the patch for this PR, I realised that
   I had made a mistake.  I should have conditionalized the limit on the
   number of supported qualifiers, so that the check is only made if we
   have resource limits enabled.  Like this:

Cheers
  Nick

Index: libiberty/cplus-dem.c
===================================================================
--- libiberty/cplus-dem.c	(revision 267043)
+++ libiberty/cplus-dem.c	(working copy)
@@ -3443,6 +3443,20 @@
       success = 0;
     }
 
+  if ((work->options & DMGL_NO_RECURSE_LIMIT) == 0)
+    {
+      /* PR 87241: Catch malicious input that will try to trick this code into
+	 allocating a ridiculous amount of memory via the remember_Ktype()
+	 function.
+	 The choice of DEMANGLE_RECURSION_LIMIT is somewhat arbitrary.  Possibly
+	 a better solution would be to track how much memory remember_Ktype
+	 allocates and abort when some upper limit is reached.  */
+      if (qualifiers > DEMANGLE_RECURSION_LIMIT)
+	/* FIXME: We ought to have some way to tell the user that
+	   this limit has been reached.  */
+	success = 0;
+    }
+
   if (!success)
     return success;
 

Re: RFA: libiberty: Add a limit on demangling qualifiers (PR 87241) (version 2)

[Ian Lance Taylor via gcc-patches]

On Wed, Dec 12, 2018 at 3:40 AM Nick Clifton <nickc@redhat.com> wrote:
>
>   *sigh* 5 minutes after sending the patch for this PR, I realised that
>    I had made a mistake.  I should have conditionalized the limit on the
>    number of supported qualifiers, so that the check is only made if we
>    have resource limits enabled.  Like this:
>
> Cheers
>   Nick
>
> Index: libiberty/cplus-dem.c
> ===================================================================
> --- libiberty/cplus-dem.c       (revision 267043)
> +++ libiberty/cplus-dem.c       (working copy)
> @@ -3443,6 +3443,20 @@
>        success = 0;
>      }
>
> +  if ((work->options & DMGL_NO_RECURSE_LIMIT) == 0)
> +    {
> +      /* PR 87241: Catch malicious input that will try to trick this code into
> +        allocating a ridiculous amount of memory via the remember_Ktype()
> +        function.
> +        The choice of DEMANGLE_RECURSION_LIMIT is somewhat arbitrary.  Possibly
> +        a better solution would be to track how much memory remember_Ktype
> +        allocates and abort when some upper limit is reached.  */
> +      if (qualifiers > DEMANGLE_RECURSION_LIMIT)
> +       /* FIXME: We ought to have some way to tell the user that
> +          this limit has been reached.  */
> +       success = 0;
> +    }
> +
>    if (!success)
>      return success;


This is OK.

Thanks.,

I thought we were removing the old demangling schemes?

Ian

Re: RFA: libiberty: Add a limit on demangling qualifiers (PR 87241) (version 2)

[Nick Clifton]

Hi Ian,

> I thought we were removing the old demangling schemes?

Doh!  yes, I totally forgot.  So I will withdraw this patch in favour of Jason's.

Cheers
  Nick