| 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() | expand |
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
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
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.