Bug 87350 - NULL-Pointer problem in cplus-dem.c when executing program c++filt
Summary: NULL-Pointer problem in cplus-dem.c when executing program c++filt
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-invalid-code
Depends on:
Blocks:
 
Reported: 2018-09-18 09:38 UTC by Cheng Wen
Modified: 2018-12-07 12:56 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-09-18 00:00:00


Attachments
POC1 (14.40 KB, text/html)
2018-09-18 09:39 UTC, Cheng Wen
Details
POC2 (14.40 KB, text/html)
2018-09-18 09:40 UTC, Cheng Wen
Details
Safe fix: Before copying work, check if the vectors have been allocated. If not, input wasn't valid. (603 bytes, text/plain)
2018-12-05 20:10 UTC, Bernhard Kaindl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Cheng Wen 2018-09-18 09:38:48 UTC
Hi,

Our fuzzer caught NULL-Pointer problems in c++filt of the latest binutils code base, those inputs will cause the segment faults and I have confirmed them with address sanitizer. 
Please use the “c++filt < $POC ” to reproduce the bug.
Please check it and debug it. Thank you.


The ASAN dumps the stack trace as follows on POC1:
https://github.com/ntu-sec/pocs/blob/master/binutils-aff4a119/crashes/npd_r_cplus-dem.c:1345_1.err.txt

AddressSanitizer:DEADLYSIGNAL
=================================================================
==23610==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f67702435a1 bp 0x7ffe2a376680 sp 0x7ffe2a375e08 T0)
==23610==The signal is caused by a READ memory access.
==23610==Hint: address points to the zero page.
    #0 0x7f67702435a0  /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/strlen-avx2.S:59
    #1 0x49728c in __interceptor_strlen.part.30 (/home/hongxu/FOT/binutils/BUILD/install/bin/c++filt+0x49728c)
    #2 0x8c9caa in work_stuff_copy_to_from /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:1345:17
    #3 0x8c553c in iterate_demangle_function /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:2731:3
    #4 0x8b77ec in demangle_prefix /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:2971:14
    #5 0x8b2d00 in internal_cplus_demangle /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:1253:14
    #6 0x8afe53 in cplus_demangle /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:918:9
    #7 0x513dd5 in demangle_it /home/hongxu/FOT/binutils/BUILD/binutils/../../binutils/cxxfilt.c:62:12
    #8 0x5139c9 in main /home/hongxu/FOT/binutils/BUILD/binutils/../../binutils/cxxfilt.c:276:4
    #9 0x7f67700d6b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #10 0x41a989 in _start (/home/hongxu/FOT/binutils/BUILD/install/bin/c++filt+0x41a989)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/strlen-avx2.S:59 
==23610==ABORTING


The ASAN dumps the stack trace as follows on POC2:
https://github.com/ntu-sec/pocs/blob/master/binutils-aff4a119/crashes/npd_r_cplus-dem.c:1360_1.err.txt

AddressSanitizer:DEADLYSIGNAL
=================================================================
==23847==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000008ca218 bp 0x7ffe44bfad50 sp 0x7ffe44bfaa10 T0)
==23847==The signal is caused by a READ memory access.
==23847==Hint: address points to the zero page.
    #0 0x8ca217 in work_stuff_copy_to_from /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:1360:25
    #1 0x8c553c in iterate_demangle_function /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:2731:3
    #2 0x8b77ec in demangle_prefix /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:2971:14
    #3 0x8b2d00 in internal_cplus_demangle /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:1253:14
    #4 0x8afe53 in cplus_demangle /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:918:9
    #5 0x513dd5 in demangle_it /home/hongxu/FOT/binutils/BUILD/binutils/../../binutils/cxxfilt.c:62:12
    #6 0x5139c9 in main /home/hongxu/FOT/binutils/BUILD/binutils/../../binutils/cxxfilt.c:276:4
    #7 0x7ff52abf2b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #8 0x41a989 in _start (/home/hongxu/FOT/binutils/BUILD/install/bin/c++filt+0x41a989)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/hongxu/FOT/binutils/BUILD/libiberty/../../libiberty/cplus-dem.c:1360:25 in work_stuff_copy_to_from
==23847==ABORTING
Comment 1 Cheng Wen 2018-09-18 09:39:58 UTC
Created attachment 44714 [details]
POC1
Comment 2 Cheng Wen 2018-09-18 09:40:22 UTC
Created attachment 44715 [details]
POC2
Comment 3 Jonathan Wakely 2018-09-18 10:28:21 UTC
(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
Comment 4 Cheng Wen 2018-09-18 10:40:01 UTC
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?
Comment 5 Bernhard Kaindl 2018-12-05 19:51:40 UTC
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.
Comment 6 Bernhard Kaindl 2018-12-05 20:10:30 UTC
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.
Comment 7 Nick Clifton 2018-12-07 10:34:02 UTC
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
Comment 8 Nick Clifton 2018-12-07 12:56:36 UTC
Fixed by commit 266886