| Message ID | 87pnu7j85i.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | RFA: libiberty: Add a limit on demangling qualifiers (PR 87241) | expand |
On Wed, Dec 12, 2018 at 6:29 AM Nick Clifton <nickc@redhat.com> wrote: > > Sorry to bother you, but I have another libiberty demangler resource > exhaustion prevention patch to present. This one is for: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87241 > > Jonathan Wakely reported that __cxa_demanlge() was returning a -2 > result, but I did not see this. Instead I found that > consume_count_with_underscores() is returning a very large number > (because a very large value is encoded in the mangled string) and this > is resulting in many calls to remember_Ktype() which eventually > exhaust the amount of memory available. > > The attached patch is a simplistic approach to solving this problem by > adding a hard upper limit on the number of qualifiers that will be > allowed by the demangler. I am not sure if this is the best approach > to solving the problem, but it is a simple one, and I would think one > that would not prevent the demangling of any real mangled names. The > limit does not have to be DEMANGLE_RECURSE_LIMIT of course. I just > chose that value because it was convenient and of a size that I > thought was appropriate. > > I also did run the libiberty testsuite this time, with no failures > reported. :-) > > OK to apply ? > > Cheers > Nick > > libiberty/ChangeLog > 2018-12-12 Nick Clifton <nickc@redhat.com> > > * cplus-dem.c (demangle_qualified): Add an upper limit on the > number of qualifiers supported, based upon the value of > DEMANGLE_RECURSE_LIMIT. This issue also will be resolved by disabling or removing the old demangling code, which I haven't seen anyone argue against. Jason
Hi Jason, > This issue also will be resolved by disabling or removing the old > demangling code, which I haven't seen anyone argue against. Doh - of course. I withdraw my patch and I hope that yours will go in soon. Cheers Nick
Index: libiberty/cplus-dem.c =================================================================== --- libiberty/cplus-dem.c (revision 267043) +++ libiberty/cplus-dem.c (working copy) @@ -3443,6 +3443,17 @@ success = 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;