| Message ID | 1520081334-5959-1-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libiberty: fix memory usage explosion for invalid templates (PR demangler/84668) | expand |
On 03/03/2018 05:48 AM, David Malcolm wrote: > PR demangler/84668 reports this failure of c++filt (found by fuzzing): > > $ c++filt '______H5z5555555555_____H5z55555555555555555555555' > c++filt: out of memory allocating 18446744071696285694 bytes after a total of 135168 bytes > > internal_cplus_demangle handles the "H5" as a template with 5 arguments; > the "z5555555555" is handled as a template parameter length of > 5555555555, though this is truncated to 32-bits to 1260588259: > > (gdb) p /x 5555555555 > $19 = 0x14b230ce3 > > (gdb) p /x r > $18 = 0x4b230ce3 > > (gdb) p r > $17 = 1260588259 > > demangle_template_template_parm repeatedly calls do_type for each of > these 1.2 billion template params, and each call attempts to handle the > "_", but hits this within demangle_fund_type: > > 3996 /* Now pick off the fundamental type. There can be only one. */ > 3997 > 3998 switch (**mangled) > 3999 { > 4000 case '\0': > 4001 case '_': > 4002 break; > > and thus returns true for success. It does this without consuming any > of the input string. > > At each iteration, it appends ", ", leading to the construction of > a string of the form: > > "____<template <, , , , , , , , , , , , , , , , , , , , , , , , , " > > and eventually the allocation fails. > > It seems like a bug for demangle_template_template_parm to be able to > arbitrarily grow like this without consuming the input string (or failing). > > This patch fixes the issue by making the NIL / '_' cases quoted above be > a failure, thus ending the iteration. I'm not sure if this is the > correct behavior (I'm new to this code), but none of the existing testcases > are affected. > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. > > OK for trunk? > > libiberty/ChangeLog: > PR demangler/84668 > * cplus-dem.c (demangle_fund_type) <'\0', '_'>: Fail. > * testsuite/demangle-expected: Add test. I kept hoping someone else would chime in here :( You should probably ping Jason directly. He's more likely to know if we can blindly return a failure at this point. Jeff
On Fri, 2018-06-22 at 15:44 -0600, Jeff Law wrote: > On 03/03/2018 05:48 AM, David Malcolm wrote: > > PR demangler/84668 reports this failure of c++filt (found by > > fuzzing): > > > > $ c++filt '______H5z5555555555_____H5z55555555555555555555555' > > c++filt: out of memory allocating 18446744071696285694 bytes after > > a total of 135168 bytes > > > > internal_cplus_demangle handles the "H5" as a template with 5 > > arguments; > > the "z5555555555" is handled as a template parameter length of > > 5555555555, though this is truncated to 32-bits to 1260588259: > > > > (gdb) p /x 5555555555 > > $19 = 0x14b230ce3 > > > > (gdb) p /x r > > $18 = 0x4b230ce3 > > > > (gdb) p r > > $17 = 1260588259 > > > > demangle_template_template_parm repeatedly calls do_type for each > > of > > these 1.2 billion template params, and each call attempts to handle > > the > > "_", but hits this within demangle_fund_type: > > > > 3996 /* Now pick off the fundamental type. There can be > > only one. */ > > 3997 > > 3998 switch (**mangled) > > 3999 { > > 4000 case '\0': > > 4001 case '_': > > 4002 break; > > > > and thus returns true for success. It does this without consuming > > any > > of the input string. > > > > At each iteration, it appends ", ", leading to the construction of > > a string of the form: > > > > "____<template <, , , , , , , , , , , , , , , , , , , , , , , , , > > " > > > > and eventually the allocation fails. > > > > It seems like a bug for demangle_template_template_parm to be able > > to > > arbitrarily grow like this without consuming the input string (or > > failing). > > > > This patch fixes the issue by making the NIL / '_' cases quoted > > above be > > a failure, thus ending the iteration. I'm not sure if this is the > > correct behavior (I'm new to this code), but none of the existing > > testcases > > are affected. > > > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. > > > > OK for trunk? > > > > libiberty/ChangeLog: > > PR demangler/84668 > > * cplus-dem.c (demangle_fund_type) <'\0', '_'>: Fail. > > * testsuite/demangle-expected: Add test. > > I kept hoping someone else would chime in here :( > > You should probably ping Jason directly. He's more likely to know if > we > can blindly return a failure at this point. > > Jeff Thanks. Jason, any thoughts? Link to original patch: https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00179.html
On Wed, Aug 29, 2018 at 9:41 AM, David Malcolm <dmalcolm@redhat.com> wrote: > On Fri, 2018-06-22 at 15:44 -0600, Jeff Law wrote: >> On 03/03/2018 05:48 AM, David Malcolm wrote: >> > PR demangler/84668 reports this failure of c++filt (found by >> > fuzzing): >> > >> > $ c++filt '______H5z5555555555_____H5z55555555555555555555555' >> > c++filt: out of memory allocating 18446744071696285694 bytes after >> > a total of 135168 bytes >> > >> > internal_cplus_demangle handles the "H5" as a template with 5 >> > arguments; >> > the "z5555555555" is handled as a template parameter length of >> > 5555555555, though this is truncated to 32-bits to 1260588259: >> > >> > (gdb) p /x 5555555555 >> > $19 = 0x14b230ce3 >> > >> > (gdb) p /x r >> > $18 = 0x4b230ce3 >> > >> > (gdb) p r >> > $17 = 1260588259 >> > >> > demangle_template_template_parm repeatedly calls do_type for each >> > of >> > these 1.2 billion template params, and each call attempts to handle >> > the >> > "_", but hits this within demangle_fund_type: >> > >> > 3996 /* Now pick off the fundamental type. There can be >> > only one. */ >> > 3997 >> > 3998 switch (**mangled) >> > 3999 { >> > 4000 case '\0': >> > 4001 case '_': >> > 4002 break; >> > >> > and thus returns true for success. It does this without consuming >> > any >> > of the input string. >> > >> > At each iteration, it appends ", ", leading to the construction of >> > a string of the form: >> > >> > "____<template <, , , , , , , , , , , , , , , , , , , , , , , , , >> > " >> > >> > and eventually the allocation fails. >> > >> > It seems like a bug for demangle_template_template_parm to be able >> > to >> > arbitrarily grow like this without consuming the input string (or >> > failing). >> > >> > This patch fixes the issue by making the NIL / '_' cases quoted >> > above be >> > a failure, thus ending the iteration. I'm not sure if this is the >> > correct behavior (I'm new to this code), but none of the existing >> > testcases >> > are affected. >> > >> > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. >> > >> > OK for trunk? >> > >> > libiberty/ChangeLog: >> > PR demangler/84668 >> > * cplus-dem.c (demangle_fund_type) <'\0', '_'>: Fail. >> > * testsuite/demangle-expected: Add test. >> >> I kept hoping someone else would chime in here :( >> >> You should probably ping Jason directly. He's more likely to know if >> we can blindly return a failure at this point. >> >> Jeff > > Thanks. > > Jason, any thoughts? The patch is OK. I'll note that this is the demangler for the GCC 2.x mangling scheme, so we might consider tearing it out at this point rather than continue to harden it against fuzzing. Jason
diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c index 6d58bd8..314746e 100644 --- a/libiberty/cplus-dem.c +++ b/libiberty/cplus-dem.c @@ -3999,6 +3999,7 @@ demangle_fund_type (struct work_stuff *work, { case '\0': case '_': + success = 0; break; case 'v': (*mangled)++; diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected index b62561c..f35aefb 100644 --- a/libiberty/testsuite/demangle-expected +++ b/libiberty/testsuite/demangle-expected @@ -4764,3 +4764,8 @@ Foo<int>()::X::fn _ZZZ3FooIiEfvENKUlT_E_clIcEEDaS0_EN1X2fnEv Foo<int>()::{lambda(auto:1)#1}::operator()<char>(char) const::X::fn() Foo<int>()::{lambda(auto:1)#1}::operator()<char>(char) const::X::fn +# demangler/84668 FIXME +--no-params +______H5z5555555555_____H5z55555555555555555555555 +______H5z5555555555_____H5z55555555555555555555555 +______H5z5555555555_____H5z55555555555555555555555
PR demangler/84668 reports this failure of c++filt (found by fuzzing): $ c++filt '______H5z5555555555_____H5z55555555555555555555555' c++filt: out of memory allocating 18446744071696285694 bytes after a total of 135168 bytes internal_cplus_demangle handles the "H5" as a template with 5 arguments; the "z5555555555" is handled as a template parameter length of 5555555555, though this is truncated to 32-bits to 1260588259: (gdb) p /x 5555555555 $19 = 0x14b230ce3 (gdb) p /x r $18 = 0x4b230ce3 (gdb) p r $17 = 1260588259 demangle_template_template_parm repeatedly calls do_type for each of these 1.2 billion template params, and each call attempts to handle the "_", but hits this within demangle_fund_type: 3996 /* Now pick off the fundamental type. There can be only one. */ 3997 3998 switch (**mangled) 3999 { 4000 case '\0': 4001 case '_': 4002 break; and thus returns true for success. It does this without consuming any of the input string. At each iteration, it appends ", ", leading to the construction of a string of the form: "____<template <, , , , , , , , , , , , , , , , , , , , , , , , , " and eventually the allocation fails. It seems like a bug for demangle_template_template_parm to be able to arbitrarily grow like this without consuming the input string (or failing). This patch fixes the issue by making the NIL / '_' cases quoted above be a failure, thus ending the iteration. I'm not sure if this is the correct behavior (I'm new to this code), but none of the existing testcases are affected. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk? libiberty/ChangeLog: PR demangler/84668 * cplus-dem.c (demangle_fund_type) <'\0', '_'>: Fail. * testsuite/demangle-expected: Add test. --- libiberty/cplus-dem.c | 1 + libiberty/testsuite/demangle-expected | 5 +++++ 2 files changed, 6 insertions(+)