Example mangled name where demangler walks past two nuls: $ echo '__thunk_16\0_\0_3foo' | c++filt virtual function thunk (delta:-16) for foo::~foo(void) These are two separate bugs: 1) The gnu_special __thunk_ handling blindly assumes (without checking) that the byte after __thunk_<num> is an underscore, and skips it: else if (strncmp (*mangled, "__thunk_", 8) == 0) { int delta; (*mangled) += 8; delta = consume_count (mangled); if (delta == -1) success = 0; else { char *method = internal_cplus_demangle (work, ++*mangled); 2) The calls to "strchr (some_chars, mangled[i]) != NULL" throughout cplus-dem.c are all wrong, as they do not properly handle the case where mangled[i] is 0. (In that case, strchr returns a *non-null* pointer to the nul terminator of some_chars.)
While we're here, this check for overflow in consume_count is nonsense, and any decent optimising compiler is going to optimise away the overflow check: https://github.com/gcc-mirror/gcc/blob/master/libiberty/cplus-dem.c#L525 Testcase: $ echo '_Z4294967297x' | c++filt x Oops. It looks like item 2 in comment#0 was fixed recently (https://github.com/gcc-mirror/gcc/commit/b2dcfe3da47412480529c8591ba0433cd495fbe3) but item 1 is still live.
For the first problem this should be sufficient: --- a/libiberty/cplus-dem.c +++ b/libiberty/cplus-dem.c @@ -3173,6 +3173,8 @@ gnu_special (struct work_stuff *work, const char **mangled, string *declp) delta = consume_count (mangled); if (delta == -1) success = 0; + else if (*mangled != '_') + success = 0; else { char *method = internal_cplus_demangle (work, ++*mangled);
Oops, no, that's not the right character to check!
(In reply to Richard Smith from comment #1) > While we're here, this check for overflow in consume_count is nonsense, and > any decent optimising compiler is going to optimise away the overflow check: > > https://github.com/gcc-mirror/gcc/blob/master/libiberty/cplus-dem.c#L525 > > Testcase: > > $ echo '_Z4294967297x' | c++filt > x > > Oops. That overflow happens in d_number in cp-demangle.c, so that check isn't used for your testcase. So we need to add a (not nonsense) check for overflow in d_number, and fix the one in consume_count.
__thunk_16__$_4294967297x would be a testcase for the consume_count overflow. I have a patch to fix these.
Author: redi Date: Thu Apr 27 09:44:28 2017 New Revision: 247300 URL: https://gcc.gnu.org/viewcvs?rev=247300&root=gcc&view=rev Log: PR demangler/80513 check for overflows and invalid characters in thunks PR demangler/80513 * cp-demangle.c (d_number): Check for overflow. * cplus-dem.c (consume_count): Fix overflow check. (gnu_special): Check for underscore after thunk delta. * testsuite/demangle-expected: Add tests for overflows and invalid characters in thunks. Modified: trunk/libiberty/ChangeLog trunk/libiberty/cp-demangle.c trunk/libiberty/cplus-dem.c trunk/libiberty/testsuite/demangle-expected
Fixed on trunk.