Bug 70481 - [Regression] Libiberty Demangler segfaults
Summary: [Regression] Libiberty Demangler segfaults
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:
: 67394 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-03-31 14:40 UTC by Marcel Böhme
Modified: 2016-05-19 12:06 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel Böhme 2016-03-31 14:40:56 UTC
In the most recent version, Valgrind reports an invalid write of size 8 due to a use-after-free if the demangler is called with a certain class signature. However, the demangling succeeds in earlier versions.

How to Reproduce:
binutils-2.26# valgrind binutils/cxxfilt _Q10-__9cafebabe.
==56086== Memcheck, a memory error detector
==56086== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==56086== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==56086== Command: binutils/cxxfilt _Q10-__9cafebabe.
==56086== 
==56086== Invalid write of size 8
==56086==    at 0x787D9B: remember_Ktype (cplus-dem.c:4300)
==56086==    by 0x787D9B: demangle_class (cplus-dem.c:2621)
==56086==    by 0x787D9B: demangle_signature (cplus-dem.c:1494)
==56086==    by 0x78DEA9: internal_cplus_demangle (cplus-dem.c:1204)
==56086==    by 0x75DC6A: cplus_demangle (cplus-dem.c:887)
==56086==    by 0x4063E1: demangle_it (cxxfilt.c:62)
==56086==    by 0x4059BE: main (cxxfilt.c:227)
..

The root cause:
There is a variable ksize storing the amount of allocated memory for the array. ksize being zero (0) indicates that some memory must be allocated upon the first write. When more memory is needed, both ksize and the memory are doubled during reallocation. At some point the memory for the array is freed but the value of ksize remains. Since ksize is not 0, there is no indication that new memory must be allocated when there is another write to the array.

The solution:
When freeing the memory of the array set ksize=0.

I am preparing a patch.
Comment 1 Mikhail Maltsev 2016-03-31 15:02:11 UTC
Likely a dup of PR67394
Comment 2 Marcel Böhme 2016-03-31 15:36:04 UTC
These are two distinct bugs. During fuzzing the btypevec bug appears more often. But it seemed less critical since only NULL is written to the freed memory:
work -> btypevec[ret] = NULL;

On the other hand, the ktypevec bug allows to write arbitrary content to the freed memory:
work -> ktypevec[work -> numk++] = tem;
where tem is "cafebabe."

I used a more efficient version of the AFL fuzzer. Interestingly, I submitted the same patch: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg01687.html
Comment 3 Jeffrey A. Law 2016-03-31 17:24:28 UTC
Fixed on the trunk.
Comment 4 Jeffrey A. Law 2016-03-31 17:25:13 UTC
*** Bug 67394 has been marked as a duplicate of this bug. ***
Comment 5 Jakub Jelinek 2016-05-19 10:45:04 UTC
Author: jakub
Date: Thu May 19 10:44:31 2016
New Revision: 236452

URL: https://gcc.gnu.org/viewcvs?rev=236452&root=gcc&view=rev
Log:
	Backported from mainline
	2016-05-19  Jakub Jelinek  <jakub@redhat.com>

	PR c++/70498
	* cp-demangle.c (d_expression_1): Formatting fix.

	2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>

	PR c++/70498
	* cp-demangle.c: Parse numbers as integer instead of long to avoid
	overflow after sanity checks. Include <limits.h> if available.
	(INT_MAX): Define if necessary.
	(d_make_template_param): Takes integer argument instead of long.
	(d_make_function_param): Likewise.
	(d_append_num): Likewise.
	(d_identifier): Likewise.
	(d_number): Parse as and return integer.
	(d_compact_number): Handle overflow.
	(d_source_name): Change variable type to integer for parsed number.
	(d_java_resource): Likewise.
	(d_special_name): Likewise.
	(d_discriminator): Likewise.
	(d_unnamed_type): Likewise.
	* testsuite/demangle-expected: Add regression test cases.

	2016-04-08  Marcel Böhme  <boehme.marcel@gmail.com>

	PR c++/69687
	* cplus-dem.c: Include <limits.h> if available.
	(INT_MAX): Define if necessary.
	(remember_type, remember_Ktype, register_Btype, string_need):
	Abort if we detect cases where we the size of the allocation would
	overflow.

	PR c++/70492
	* cplus-dem.c (gnu_special): Handle case where consume_count returns
	-1.

	2016-03-31  Mikhail Maltsev  <maltsevm@gmail.com>
		    Marcel Bohme  <boehme.marcel@gmail.com>

	PR c++/67394
	PR c++/70481
	* cplus-dem.c (squangle_mop_up): Zero bsize/ksize after freeing
	btypevec/ktypevec.
	* testsuite/demangle-expected: Add coverage tests.

Modified:
    branches/gcc-5-branch/libiberty/ChangeLog
    branches/gcc-5-branch/libiberty/cp-demangle.c
    branches/gcc-5-branch/libiberty/cplus-dem.c
    branches/gcc-5-branch/libiberty/testsuite/demangle-expected
Comment 6 Jakub Jelinek 2016-05-19 12:06:14 UTC
Author: jakub
Date: Thu May 19 12:05:41 2016
New Revision: 236456

URL: https://gcc.gnu.org/viewcvs?rev=236456&root=gcc&view=rev
Log:
	Backported from mainline
	2016-05-19  Jakub Jelinek  <jakub@redhat.com>

	PR c++/70498
	* cp-demangle.c (d_expression_1): Formatting fix.

	2016-05-02  Marcel Böhme  <boehme.marcel@gmail.com>

	PR c++/70498
	* cp-demangle.c: Parse numbers as integer instead of long to avoid
	overflow after sanity checks. Include <limits.h> if available.
	(INT_MAX): Define if necessary.
	(d_make_template_param): Takes integer argument instead of long.
	(d_make_function_param): Likewise.
	(d_append_num): Likewise.
	(d_identifier): Likewise.
	(d_number): Parse as and return integer.
	(d_compact_number): Handle overflow.
	(d_source_name): Change variable type to integer for parsed number.
	(d_java_resource): Likewise.
	(d_special_name): Likewise.
	(d_discriminator): Likewise.
	(d_unnamed_type): Likewise.
	* testsuite/demangle-expected: Add regression test cases.

	2016-04-08  Marcel Böhme  <boehme.marcel@gmail.com>

	PR c++/69687
	* cplus-dem.c: Include <limits.h> if available.
	(INT_MAX): Define if necessary.
	(remember_type, remember_Ktype, register_Btype, string_need):
	Abort if we detect cases where we the size of the allocation would
	overflow.

	PR c++/70492
	* cplus-dem.c (gnu_special): Handle case where consume_count returns
	-1.

	2016-03-31  Mikhail Maltsev  <maltsevm@gmail.com>
		    Marcel Bohme  <boehme.marcel@gmail.com>

	PR c++/67394
	PR c++/70481
	* cplus-dem.c (squangle_mop_up): Zero bsize/ksize after freeing
	btypevec/ktypevec.
	* testsuite/demangle-expected: Add coverage tests.

Modified:
    branches/gcc-4_9-branch/libiberty/ChangeLog
    branches/gcc-4_9-branch/libiberty/cp-demangle.c
    branches/gcc-4_9-branch/libiberty/cplus-dem.c
    branches/gcc-4_9-branch/libiberty/testsuite/demangle-expected