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

Message ID 87mupbj7mk.fsf@redhat.com
State New
Headers show
Series
  • RFA: libiberty: Add a limit on demangling qualifiers (PR 87241) (version 2)
Related show

Commit Message

Nick Clifton Dec. 12, 2018, 11:40 a.m.
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

Comments

Jose E. Marchesi via Gcc-patches Dec. 12, 2018, 1:47 p.m. | #1
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
Nick Clifton Dec. 13, 2018, 9:47 a.m. | #2
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

Patch

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;