Created attachment 44714 [details]
POC1

Created attachment 44715 [details]
POC2

(In reply to Cheng Wen from comment #1)
> Created attachment 44714 [details]
> POC1

You've uploaded two complete HTML pages saved from github, but the mangled name that crash are just:

_GLOBAL_$D$__tf30___0__
__thunk_0__0__$__H1



$ echo '_GLOBAL_$D$__tf30___0__' | /tmp/binutils/bin/c++filt 
ASAN:DEADLYSIGNAL
=================================================================
==6977==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f5fbbb47f31 bp 0x7fff4a202c20 sp 0x7fff4a202398 T0)
==6977==The signal is caused by a READ memory access.
==6977==Hint: address points to the zero page.
    #0 0x7f5fbbb47f30 in __strlen_avx2 (/lib64/libc.so.6+0x155f30)
    #1 0x7f5fbbffd27b  (/lib64/libasan.so.4+0x5127b)
    #2 0x497e34 in work_stuff_copy_to_from /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:1345
    #3 0x49cdd8 in iterate_demangle_function /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:2731
    #4 0x49d962 in demangle_prefix /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:2971
    #5 0x49d962 in internal_cplus_demangle /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:1253
    #6 0x498860 in cplus_demangle /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:918
    #7 0x402ea5 in demangle_it (/tmp/binutils/bin/c++filt+0x402ea5)
    #8 0x4037af in main (/tmp/binutils/bin/c++filt+0x4037af)
    #9 0x7f5fbba12fe9 in __libc_start_main (/lib64/libc.so.6+0x20fe9)
    #10 0x402d29 in _start (/tmp/binutils/bin/c++filt+0x402d29)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x155f30) in __strlen_avx2
==6977==ABORTING
wraith:tmp$ echo '__thunk_0__0__$__H1' | /tmp/binutils/bin/c++filt 
ASAN:DEADLYSIGNAL
=================================================================
==6981==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000497f27 bp 0x7ffc897891e0 sp 0x7ffc89789170 T0)
==6981==The signal is caused by a READ memory access.
==6981==Hint: address points to the zero page.
    #0 0x497f26 in work_stuff_copy_to_from /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:1358
    #1 0x49cdd8 in iterate_demangle_function /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:2731
    #2 0x49d962 in demangle_prefix /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:2971
    #3 0x49d962 in internal_cplus_demangle /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:1253
    #4 0x498860 in cplus_demangle /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:918
    #5 0x402ea5 in demangle_it (/tmp/binutils/bin/c++filt+0x402ea5)
    #6 0x4037af in main (/tmp/binutils/bin/c++filt+0x4037af)
    #7 0x7ff5a9a18fe9 in __libc_start_main (/lib64/libc.so.6+0x20fe9)
    #8 0x402d29 in _start (/tmp/binutils/bin/c++filt+0x402d29)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/jwakely/src/binutils-gdb/libiberty/cplus-dem.c:1358 in work_stuff_copy_to_from
==6981==ABORTING

Yes.

One input test case is "_GLOBAL_$D$__tf30___0__".
Another input test case is "__thunk_0__0__$__H1".

I see that you can you can reproduce this error. Do you know the reason for this bug?

Simple observation from the asan traces, the source and a quick test:

iterate_demangle_function (struct work_stuff *work, ...
{
   [only cases for early returns here]

   work_stuff_copy_to_from (&work_init, work); <- SEGV on a member of work here

work_stuff_copy_to_from () is a dumb copy function which assumes that the vectors it shall copy have been allocated:

void work_stuff_copy_to_from (struct work_stuff *to, struct work_stuff *from)

  [nothing relevant here]
  for... {
      int len = strlen (from->ktypevec[i]) + 1; <- SIGSEGV happens here bc NULL.

I verified that function remember_Ktype() which does the all allocation of work_stuff->ktypevec is never called by these POCs, hence ktypevec is still NULL, causing the SIGSEGV.

iterate_demangle_function() itself is called from a rather complex function.

The only safe fix: Before copying work_stuff, check the work_stuff vectors to be already. If not, return 0 -> no demangle.

Created attachment 45167 [details]
Safe fix: Before copying work, check if the vectors have been allocated. If not, input wasn't valid.

Fixes CVE-2018-17794:
  
In cplus-dem.c in GNU libiberty, as distributed in GNU Binutils 2.31 (and all
prior versions) There is a NULL pointer dereference in work_stuff_copy_to_from
when called from iterate_demangle_function.

https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2018-17794

Safe fix: Before copying work, check if the vectors have been allocated.
If not, input wasn't valid. -- Bernhard Kaindl

diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index 6d58bd899b..ab30cd5fd5 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -2723,6 +2723,11 @@ iterate_demangle_function (struct work_stuff *work, const char **mangled,
       || strstr (scan + 2, "__") == NULL)
     return demangle_function_name (work, mangled, declp, scan);

+  /* Before copying work, check if the vectors have been allocated.
+     If not, our input isn't a valid mangled name and we'd sigseg then: */
+  if (!work->typevec || !work->ktypevec || !work->btypevec)
+    return 0;
+
   /* Save state so we can restart if the guess at the correct "__" was
      wrong.  */
   string_init (&decl_init);

It would be enough to check just for !work->typevec to fix this CVE, the others are just related as work_stuff_copy_to_from() copies them in the same way as ktypevec.

To be sure there is no oversight, proper review and testing would be in required.

Author: nickc
Date: Fri Dec  7 10:33:30 2018
New Revision: 266886

URL: https://gcc.gnu.org/viewcvs?rev=266886&root=gcc&view=rev
Log:
Add a recursion limit to libiberty's demangling code.  The limit is enabled by default, but can be disabled via a new demangling option.

include	* demangle.h (DMGL_NO_RECURSE_LIMIT): Define.
        (DEMANGLE_RECURSION_LIMIT): Define

	PR 87681
	PR 87675
	PR 87636
	PR 87350
	PR 87335
libiberty * cp-demangle.h (struct d_info): Add recursion_level field.
	* cp-demangle.c (d_function_type): Add recursion counter.
	If the recursion limit is reached and the check is not disabled,
	then return with a failure result.
	(cplus_demangle_init_info): Initialise the recursion_level field.
        (d_demangle_callback): If the recursion limit is enabled, check
	for a mangled string that is so long that there is not enough
	stack space for the local arrays.
        * cplus-dem.c (struct work): Add recursion_level field.
	(squangle_mop_up): Set the numb and numk fields to zero.
	(work_stuff_copy_to_from): Handle the case where a btypevec or 
	ktypevec field is NULL.
	(demangle_nested_args): Add recursion counter.  If
	the recursion limit is not disabled and reached, return with a
	failure result.

Modified:
    trunk/include/ChangeLog
    trunk/include/demangle.h
    trunk/libiberty/ChangeLog
    trunk/libiberty/cp-demangle.c
    trunk/libiberty/cp-demangle.h
    trunk/libiberty/cplus-dem.c

Fixed by commit 266886