diff mbox series

[libiberty] Fix write buffer overflow in cplus_demangle

Message ID 20191129191558.5372-1-tim.ruehsen@gmx.de
State New
Headers show
Series [libiberty] Fix write buffer overflow in cplus_demangle | expand

Commit Message

Tim Rühsen Nov. 29, 2019, 7:15 p.m. UTC
* cplus-dem.c (ada_demangle): Correctly calculate the demangled
  size by using two passes.

Fixes #92453
---
 libiberty/ChangeLog   |   5 +
 libiberty/cplus-dem.c | 408 +++++++++++++++++++++++-------------------
 2 files changed, 226 insertions(+), 187 deletions(-)

--
2.24.0

Comments

Jeff Law Nov. 13, 2020, 4:45 a.m. UTC | #1
On 11/29/19 12:15 PM, Tim Rühsen wrote:
> * cplus-dem.c (ada_demangle): Correctly calculate the demangled
>   size by using two passes.

So I'm not sure why, but I can't get this patch to apply.  What's even
more interesting is ada_demangle doesn't seem to have changed since 2010
and even if I checkout a Nov 2019 trunk, I still can't apply the patch.


I can see what you're doing with your patch (it's primarily introducing
a loop where you count on the first pass and allocate on the second and
re-indent all the necessary code), I'd prefer not to muck it up trying
to apply by hand.


Any change you could update the patch so that it applies to the trunk. 
THe review is done, so it should be able to go straight in.  If you have
commit privs (I don't recall if you do or not), you can go ahead and
commit it yourself.


Sorry for the insane delays here.

jeff
Tim Rühsen Nov. 14, 2020, 1:08 p.m. UTC | #2
Hey,

On 13.11.20 05:45, Jeff Law wrote:
> 
> On 11/29/19 12:15 PM, Tim Rühsen wrote:
>> * cplus-dem.c (ada_demangle): Correctly calculate the demangled
>>    size by using two passes.
> 
> So I'm not sure why, but I can't get this patch to apply.  What's even
> more interesting is ada_demangle doesn't seem to have changed since 2010
> and even if I checkout a Nov 2019 trunk, I still can't apply the patch.
> 
> 
> I can see what you're doing with your patch (it's primarily introducing
> a loop where you count on the first pass and allocate on the second and
> re-indent all the necessary code), I'd prefer not to muck it up trying
> to apply by hand.
> 
> 
> Any change you could update the patch so that it applies to the trunk.
> THe review is done, so it should be able to go straight in.  If you have
> commit privs (I don't recall if you do or not), you can go ahead and
> commit it yourself.

hm sorry, I am a bit out of the loop currently. It would be awesome if 
someone with more project knowledge could apply the patch.

 From what I can see here, the patch was made on top of binutils-gdb 
commit 3d18c3354209bd42361cb26ec611455cdf8b401b. Hope this helps.

> Sorry for the insane delays here.

That is how life goes ;-)
A delay is better than never.

Regards, Tim
Jeff Law Nov. 18, 2020, 9:32 p.m. UTC | #3
On 11/14/20 6:08 AM, Tim Rühsen wrote:
> Hey,
>
> On 13.11.20 05:45, Jeff Law wrote:
>>
>> On 11/29/19 12:15 PM, Tim Rühsen wrote:
>>> * cplus-dem.c (ada_demangle): Correctly calculate the demangled
>>>    size by using two passes.
>>
>> So I'm not sure why, but I can't get this patch to apply.  What's even
>> more interesting is ada_demangle doesn't seem to have changed since 2010
>> and even if I checkout a Nov 2019 trunk, I still can't apply the patch.
>>
>>
>> I can see what you're doing with your patch (it's primarily introducing
>> a loop where you count on the first pass and allocate on the second and
>> re-indent all the necessary code), I'd prefer not to muck it up trying
>> to apply by hand.
>>
>>
>> Any change you could update the patch so that it applies to the trunk.
>> THe review is done, so it should be able to go straight in.  If you have
>> commit privs (I don't recall if you do or not), you can go ahead and
>> commit it yourself.
>
> hm sorry, I am a bit out of the loop currently. It would be awesome if
> someone with more project knowledge could apply the patch.
>
> From what I can see here, the patch was made on top of binutils-gdb
> commit 3d18c3354209bd42361cb26ec611455cdf8b401b. Hope this helps.
Normally a GIT id would be sufficient...  *But*:
[law@localhost binutils-gdb]$ git checkout
3d18c3354209bd42361cb26ec611455cdf8b401b
fatal: reference is not a tree: 3d18c3354209bd42361cb26ec611455cdf8b401b

Maybe you could send me your cplus-dem.c with and without your patch
installed.  I can probably sort it out from there.



>
>> Sorry for the insane delays here.
>
> That is how life goes ;-)
> A delay is better than never.
Yea, but it shouldn't take this long to get to relatively simple
patches.  It's just been a terrible year.

jeff
diff mbox series

Patch

diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 95cb1525f2..085b7b3fbf 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-11-16  Tim Ruehsen  <tim.ruehsen@gmx.de>
+
+	* cplus-dem.c (ada_demangle): Correctly calculate the demangled
+	size by using two passes. Fixes #92453.
+
 2019-08-08  Martin Liska  <mliska@suse.cz>

 	PR bootstrap/91352
diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index a39e2bf2ed..be76e691b0 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -230,7 +230,7 @@  rust_demangle (const char *mangled, int options)
 char *
 ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
 {
-  int len0;
+  int len0 = 0, run;
   const char* p;
   char *d;
   char *demangled = NULL;
@@ -243,236 +243,270 @@  ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
   if (!ISLOWER (mangled[0]))
     goto unknown;

-  /* Most of the demangling will trivially remove chars.  Operator names
-     may add one char but because they are always preceeded by '__' which is
-     replaced by '.', they eventually never expand the size.
-     A few special names such as '___elabs' add a few chars (at most 7), but
-     they occur only once.  */
-  len0 = strlen (mangled) + 7 + 1;
-  demangled = XNEWVEC (char, len0);
-
-  d = demangled;
-  p = mangled;
-  while (1)
+  for (run = 0; run <= 1; run++)
     {
-      /* An entity names is expected.  */
-      if (ISLOWER (*p))
+      if (run == 1)
         {
-          /* An identifier, which is always lower case.  */
-          do
-            *d++ = *p++;
-          while (ISLOWER(*p) || ISDIGIT (*p)
-                 || (p[0] == '_' && (ISLOWER (p[1]) || ISDIGIT (p[1]))));
+          demangled = XNEWVEC (char, len0 + 1);
         }
-      else if (p[0] == 'O')
+
+      p = mangled;
+      d = demangled;
+
+      while (1)
         {
-          /* An operator name.  */
-          static const char * const operators[][2] =
-            {{"Oabs", "abs"},  {"Oand", "and"},    {"Omod", "mod"},
-             {"Onot", "not"},  {"Oor", "or"},      {"Orem", "rem"},
-             {"Oxor", "xor"},  {"Oeq", "="},       {"One", "/="},
-             {"Olt", "<"},     {"Ole", "<="},      {"Ogt", ">"},
-             {"Oge", ">="},    {"Oadd", "+"},      {"Osubtract", "-"},
-             {"Oconcat", "&"}, {"Omultiply", "*"}, {"Odivide", "/"},
-             {"Oexpon", "**"}, {NULL, NULL}};
-          int k;
-
-          for (k = 0; operators[k][0] != NULL; k++)
+          /* An entity names is expected.  */
+          if (ISLOWER (*p))
             {
-              size_t slen = strlen (operators[k][0]);
-              if (strncmp (p, operators[k][0], slen) == 0)
+              /* An identifier, which is always lower case.  */
+              do
+                run ? (*d++ = *p++) : (p++, len0++);
+              while (ISLOWER (*p) || ISDIGIT (*p)
+                     || (p[0] == '_' && (ISLOWER (p[1]) || ISDIGIT (p[1]))));
+            }
+          else if (p[0] == 'O')
+            {
+              /* An operator name.  */
+              static const char * const operators[][2] ={
+                {"Oabs", "abs"},
+                {"Oand", "and"},
+                {"Omod", "mod"},
+                {"Onot", "not"},
+                {"Oor", "or"},
+                {"Orem", "rem"},
+                {"Oxor", "xor"},
+                {"Oeq", "="},
+                {"One", "/="},
+                {"Olt", "<"},
+                {"Ole", "<="},
+                {"Ogt", ">"},
+                {"Oge", ">="},
+                {"Oadd", "+"},
+                {"Osubtract", "-"},
+                {"Oconcat", "&"},
+                {"Omultiply", "*"},
+                {"Odivide", "/"},
+                {"Oexpon", "**"},
+                {NULL, NULL}};
+              int k;
+
+              for (k = 0; operators[k][0] != NULL; k++)
                 {
-                  p += slen;
-                  slen = strlen (operators[k][1]);
-                  *d++ = '"';
-                  memcpy (d, operators[k][1], slen);
-                  d += slen;
-                  *d++ = '"';
-                  break;
+                  size_t slen = strlen (operators[k][0]);
+                  if (strncmp (p, operators[k][0], slen) == 0)
+                    {
+                      p += slen;
+                      slen = strlen (operators[k][1]);
+                      if (run)
+                        {
+                          *d++ = '"';
+                          memcpy (d, operators[k][1], slen);
+                          d += slen;
+                          *d++ = '"';
+                        }
+                      else
+                        len0 += slen + 2;
+                      break;
+                    }
                 }
+              /* Operator not found.  */
+              if (operators[k][0] == NULL)
+                goto unknown;
             }
-          /* Operator not found.  */
-          if (operators[k][0] == NULL)
-            goto unknown;
-        }
-      else
-        {
-          /* Not a GNAT encoding.  */
-          goto unknown;
-        }
-
-      /* The name can be directly followed by some uppercase letters.  */
-      if (p[0] == 'T' && p[1] == 'K')
-        {
-          /* Task stuff.  */
-          if (p[2] == 'B' && p[3] == 0)
+          else
             {
-              /* Subprogram for task body.  */
-              break;
+              /* Not a GNAT encoding.  */
+              goto unknown;
             }
-          else if (p[2] == '_' && p[3] == '_')
+
+          /* The name can be directly followed by some uppercase letters.  */
+          if (p[0] == 'T' && p[1] == 'K')
             {
-              /* Inner declarations in a task.  */
-              p += 4;
-              *d++ = '.';
-              continue;
+              /* Task stuff.  */
+              if (p[2] == 'B' && p[3] == 0)
+                {
+                  /* Subprogram for task body.  */
+                  break;
+                }
+              else if (p[2] == '_' && p[3] == '_')
+                {
+                  /* Inner declarations in a task.  */
+                  p += 4;
+                  run ? *d++ = '.' : len0++;
+                  continue;
+                }
+              else
+                goto unknown;
             }
-          else
-            goto unknown;
-        }
-      if (p[0] == 'E' && p[1] == 0)
-        {
-          /* Exception name.  */
-          goto unknown;
-        }
-      if ((p[0] == 'P' || p[0] == 'N') && p[1] == 0)
-        {
-          /* Protected type subprogram.  */
-          break;
-        }
-      if ((*p == 'N' || *p == 'S') && p[1] == 0)
-        {
-          /* Enumerated type name table.  */
-          goto unknown;
-        }
-      if (p[0] == 'X')
-        {
-          /* Body nested.  */
-          p++;
-          while (p[0] == 'n' || p[0] == 'b')
-            p++;
-        }
-      if (p[0] == 'S' && p[1] != 0 && (p[2] == '_' || p[2] == 0))
-        {
-          /* Stream operations.  */
-          const char *name;
-          switch (p[1])
+          if (p[0] == 'E' && p[1] == 0)
             {
-            case 'R':
-              name = "'Read";
-              break;
-            case 'W':
-              name = "'Write";
-              break;
-            case 'I':
-              name = "'Input";
-              break;
-            case 'O':
-              name = "'Output";
-              break;
-            default:
+              /* Exception name.  */
               goto unknown;
             }
-          p += 2;
-          strcpy (d, name);
-          d += strlen (name);
-        }
-      else if (p[0] == 'D')
-        {
-          /* Controlled type operation.  */
-          const char *name;
-          switch (p[1])
+          if ((p[0] == 'P' || p[0] == 'N') && p[1] == 0)
             {
-            case 'F':
-              name = ".Finalize";
-              break;
-            case 'A':
-              name = ".Adjust";
+              /* Protected type subprogram.  */
               break;
-            default:
+            }
+          if ((*p == 'N' || *p == 'S') && p[1] == 0)
+            {
+              /* Enumerated type name table.  */
               goto unknown;
             }
-          strcpy (d, name);
-          d += strlen (name);
-          break;
-        }
-
-      if (p[0] == '_')
-        {
-          /* Separator.  */
-          if (p[1] == '_')
+          if (p[0] == 'X')
+            {
+              /* Body nested.  */
+              p++;
+              while (p[0] == 'n' || p[0] == 'b')
+                p++;
+            }
+          if (p[0] == 'S' && p[1] != 0 && (p[2] == '_' || p[2] == 0))
             {
-              /* Standard separator.  Handled first.  */
+              /* Stream operations.  */
+              const char *name;
+              switch (p[1])
+                {
+                case 'R':
+                  name = "'Read";
+                  break;
+                case 'W':
+                  name = "'Write";
+                  break;
+                case 'I':
+                  name = "'Input";
+                  break;
+                case 'O':
+                  name = "'Output";
+                  break;
+                default:
+                  goto unknown;
+                }
               p += 2;
+              if (run)
+                {
+                  strcpy (d, name);
+                  d += strlen (name);
+                }
+              else
+                len0 += strlen (name);
+            }
+          else if (p[0] == 'D')
+            {
+              /* Controlled type operation.  */
+              const char *name;
+              switch (p[1])
+                {
+                case 'F':
+                  name = ".Finalize";
+                  break;
+                case 'A':
+                  name = ".Adjust";
+                  break;
+                default:
+                  goto unknown;
+                }
+              if (run)
+                {
+                  strcpy (d, name);
+                  d += strlen (name);
+                }
+              else
+                len0 += strlen (name);
+              break;
+            }

-              if (ISDIGIT (*p))
+          if (p[0] == '_')
+            {
+              /* Separator.  */
+              if (p[1] == '_')
                 {
-                  /* Overloading number.  */
-                  do
-                    p++;
-                  while (ISDIGIT (*p) || (p[0] == '_' && ISDIGIT (p[1])));
-                  if (*p == 'X')
+                  /* Standard separator.  Handled first.  */
+                  p += 2;
+
+                  if (ISDIGIT (*p))
                     {
-                      p++;
-                      while (p[0] == 'n' || p[0] == 'b')
+                      /* Overloading number.  */
+                      do
                         p++;
+                      while (ISDIGIT (*p) || (p[0] == '_' && ISDIGIT (p[1])));
+                      if (*p == 'X')
+                        {
+                          p++;
+                          while (p[0] == 'n' || p[0] == 'b')
+                            p++;
+                        }
                     }
-                }
-              else if (p[0] == '_' && p[1] != '_')
-                {
-                  /* Special names.  */
-                  static const char * const special[][2] = {
-                    { "_elabb", "'Elab_Body" },
-                    { "_elabs", "'Elab_Spec" },
-                    { "_size", "'Size" },
-                    { "_alignment", "'Alignment" },
-                    { "_assign", ".\":=\"" },
-                    { NULL, NULL }
-                  };
-                  int k;
-
-                  for (k = 0; special[k][0] != NULL; k++)
+                  else if (p[0] == '_' && p[1] != '_')
                     {
-                      size_t slen = strlen (special[k][0]);
-                      if (strncmp (p, special[k][0], slen) == 0)
+                      /* Special names.  */
+                      static const char * const special[][2] = {
+                        { "_elabb", "'Elab_Body"},
+                        { "_elabs", "'Elab_Spec"},
+                        { "_size", "'Size"},
+                        { "_alignment", "'Alignment"},
+                        { "_assign", ".\":=\""},
+                        { NULL, NULL}
+                      };
+                      int k;
+
+                      for (k = 0; special[k][0] != NULL; k++)
                         {
-                          p += slen;
-                          slen = strlen (special[k][1]);
-                          memcpy (d, special[k][1], slen);
-                          d += slen;
-                          break;
+                          size_t slen = strlen (special[k][0]);
+                          if (strncmp (p, special[k][0], slen) == 0)
+                            {
+                              p += slen;
+                              slen = strlen (special[k][1]);
+                              if (run)
+                                {
+                                  memcpy (d, special[k][1], slen);
+                                  d += slen;
+                                }
+                              else
+                                len0 += slen;
+                              break;
+                            }
                         }
+                      if (special[k][0] != NULL)
+                        break;
+                      else
+                        goto unknown;
+                    }
+                  else
+                    {
+                      run ? *d++ = '.' : len0++;
+                      continue;
                     }
-                  if (special[k][0] != NULL)
+                }
+              else if (p[1] == 'B' || p[1] == 'E')
+                {
+                  /* Entry Body or barrier Evaluation.  */
+                  p += 2;
+                  while (ISDIGIT (*p))
+                    p++;
+                  if (p[0] == 's' && p[1] == 0)
                     break;
                   else
                     goto unknown;
                 }
               else
-                {
-                  *d++ = '.';
-                  continue;
-                }
+                goto unknown;
             }
-          else if (p[1] == 'B' || p[1] == 'E')
+
+          if (p[0] == '.' && ISDIGIT (p[1]))
             {
-              /* Entry Body or barrier Evaluation.  */
+              /* Nested subprogram.  */
               p += 2;
               while (ISDIGIT (*p))
                 p++;
-              if (p[0] == 's' && p[1] == 0)
-                break;
-              else
-                goto unknown;
+            }
+          if (*p == 0)
+            {
+              /* End of mangled name.  */
+              break;
             }
           else
             goto unknown;
         }
-
-      if (p[0] == '.' && ISDIGIT (p[1]))
-        {
-          /* Nested subprogram.  */
-          p += 2;
-          while (ISDIGIT (*p))
-            p++;
-        }
-      if (*p == 0)
-        {
-          /* End of mangled name.  */
-          break;
-        }
-      else
-        goto unknown;
     }
   *d = 0;
   return demangled;