[04/10] libiberty: Fix crash in ada_demangle()

Message ID CY4PR22MB01025673FB46C1C6919B8DE1E7850@CY4PR22MB0102.namprd22.prod.outlook.com
State New
Headers show
Series
  • [01/10] libiberty: Fix an out of bounds read in d_expression_1()
Related show

Commit Message

Ben L Jan. 11, 2019, 12:16 a.m.
Hi all,

First time emailing gcc-patches, so I'm sorry if I get any of this wrong or if
there's obvious errors repeated in my patches. AFAICT I should be sending each
change individually rather than as one bulk patch, so I'm sorry about the spam
too.

All of these changes were found by fuzzing libiberty's demanglers over the
past week, and I have at least one more that it's currently crashing out on
but I haven't had time to look into why yet.

Obviously since this is my first time emailing I don't have write access to
commit any of these, so if any are approved then I'd be grateful if you can
commit them too.

Thanks,
Ben

--

The output buffer is pre-allocated to a maximum size under the assumption that
special names can only occur once, however nothing was enforcing this for
stream attributes.

To fix this we treat stream attributes that appear before the end of the
mangled input as an error.

     * cplus-dem.c (ada_demangle): Only accept stream attributes if they're at
     the end of the input.
     * testsuite/demangle-expected: Add testcase.

Comments

Jeff Law April 30, 2019, 2:45 p.m. | #1
On 1/10/19 5:16 PM, Ben L wrote:
> Hi all,

> 

> First time emailing gcc-patches, so I'm sorry if I get any of this wrong or if

> there's obvious errors repeated in my patches. AFAICT I should be sending each

> change individually rather than as one bulk patch, so I'm sorry about the spam

> too.

> 

> All of these changes were found by fuzzing libiberty's demanglers over the

> past week, and I have at least one more that it's currently crashing out on

> but I haven't had time to look into why yet.

> 

> Obviously since this is my first time emailing I don't have write access to

> commit any of these, so if any are approved then I'd be grateful if you can

> commit them too.

> 

> Thanks,

> Ben

> 

> --

> 

> The output buffer is pre-allocated to a maximum size under the assumption that

> special names can only occur once, however nothing was enforcing this for

> stream attributes.

> 

> To fix this we treat stream attributes that appear before the end of the

> mangled input as an error.

> 

>      * cplus-dem.c (ada_demangle): Only accept stream attributes if they're at

>      the end of the input.

>      * testsuite/demangle-expected: Add testcase.

> 

I don't feel qualified enough to ACK/NACK this patch.  What's not clear
to me is whether or not a stream attribute can or can not appear more
than once in a mangled name in Ada -- largely because of my lack of
knowledge of Ada itself.

Are function names mangled in Ada?  If so, couldn't we have multiple
parameters, each of which potentially has a stream attribute?

What about structures where there's multiple members which have stream
attributes?  If we then have to mangle the structure, then ISTM we could
end up with multiple stream attributes.

Someone with more experience in Ada and our mangling scheme will need to
chime in.

Jeff

Patch

From c8dd053c841e9b04583ad6c6bf4550d30aa47990 Mon Sep 17 00:00:00 2001
From: bobsayshilol <bobsayshilol@live.co.uk>
Date: Wed, 9 Jan 2019 22:18:14 +0000
Subject: [PATCH 04/10] libiberty: Fix crash in ada_demangle().

The output buffer is pre-allocated to a maximum size under the assumption that
special names can only occur once, however nothing was enforcing this for
stream attributes.

To fix this we treat stream attributes that appear before the end of the
mangled input as an error.

    * cplus-dem.c (ada_demangle): Only accept stream attributes if they're at
    the end of the input.
    * testsuite/demangle-expected: Add testcase.

diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index afceed2..245cf11 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -254,6 +254,8 @@  ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
   p = mangled;
   while (1)
     {
+      int stream = 0;
+
       /* An entity names is expected.  */
       if (ISLOWER (*p))
         {
@@ -363,6 +365,7 @@  ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
               goto unknown;
             }
           p += 2;
+          stream = 1;
           strcpy (d, name);
           d += strlen (name);
         }
@@ -437,6 +440,10 @@  ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
                   else
                     goto unknown;
                 }
+              else if (stream)
+                {
+                  goto unknown;
+                }
               else
                 {
                   *d++ = '.';
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index f21ed00..8b830b6 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -81,6 +81,10 @@  _ZZaSFvOEES_
 
 _ZZeqFvOEES_z
 _ZZeqFvOEES_z
+# Could crash
+--format=gnat
+lSO__lSO
+<lSO__lSO>
 #
 # demangler/80513 Test for bogus characters after __thunk_
 
-- 
2.20.1