Bug 69687 - Buffer Overflow in libiberty
Summary: Buffer Overflow in libiberty
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:
Depends on:
Blocks:
 
Reported: 2016-02-05 10:41 UTC by Marcel Böhme
Modified: 2016-05-19 12:06 UTC (History)
4 users (show)

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


Attachments
Test Case #1 (71 bytes, application/octet-stream)
2016-02-05 10:41 UTC, Marcel Böhme
Details
Test Case #2 (185 bytes, application/octet-stream)
2016-02-05 10:42 UTC, Marcel Böhme
Details
Debug This (118 bytes, text/x-csrc)
2016-02-06 17:20 UTC, Marcel Böhme
Details
Valgrind This (2.49 KB, application/x-executable)
2016-02-07 07:04 UTC, Marcel Böhme
Details
Proposed Patch (441 bytes, patch)
2016-03-02 08:02 UTC, Marcel Böhme
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel Böhme 2016-02-05 10:41:56 UTC
Created attachment 37592 [details]
Test Case #1

The attached program binary causes a buffer overflow in cplus-dem.c when it tries to demangle specially crafted function arguments in the binary. Both the buffer size as well as the buffer content are controlled from the binary.

objdump -x -C <file>
nm -C <file>

Tested on the following configurations
* 2.6.32-573.7.1.el6.x86_64 #1 SMP Tue Sep 22 22:00:00 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
* 4.1.12-boot2docker #1 SMP Tue Nov 3 06:03:36 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
* Binutils versions: 2.20 and 2.26

Best regards,
- Marcel

Backlink: https://sourceware.org/bugzilla/show_bug.cgi?id=19571
Comment 1 Marcel Böhme 2016-02-05 10:42:31 UTC
Created attachment 37593 [details]
Test Case #2
Comment 2 Markus Trippelsdorf 2016-02-05 10:52:06 UTC
I don't think it makes much sense to fuzz the demangler with arbitrary binary files.
This isn't some daemon that runs 24/7 and is vulnerable to buffer overflow attacks.
Comment 3 Marcel Böhme 2016-02-05 11:18:18 UTC
Hi Markus,

Indeed, it depends on the use case. I find it quite unsettling to know that common digital forensics tools, such as gdb and objdump, are vulnerable to execute arbitrary code.

How much credibility would a virus scanner have that installs a virus only because it *scanned* a malicious software?

Best regards,
- Marcel
Comment 4 Marcel Böhme 2016-02-05 16:07:16 UTC
Here is my preliminary analysis:
The function demangle_args (cplus-dem.c:4424) parses the (crafted) mangled function args from the binary. In line 4452, r is read from mangled. In line 4491, we enter a loop with r iterations. In line 4498, arg is parsed from mangled (always from the same region). In each loop iteration arg is appended to decl, such that the length of decl grows in each iteration.

The function string_appends (cplus-dem.c:4820) appends string arg to string decl. In line 4827, it uses string_need to check whether sufficient memory is allocated in decl to append arg. Then, it memcopies whatever is in arg to the end of decl.

The function string_need (cplus-dem.c:4751) checks whether sufficient memory is available to append size-of-arg more characters. If not, xrealloc decl with n=2*(length of decl + length of arg) characters. Since n is a signed int, n wraps over at some iteration. Since, realloc expects n to be unsigned, we end up allocating less memory then actually needed. In the beginning though n is too large and xrealloc simply complains. However, if you play a bit with the length of arg, you'll quickly turn that integer overflow in a buffer overflow. Later, string_appends will memcopy whatever arg contains.

In our simple test case the relevant bits in the binary are specified the binary (arbitrarily): r = 80000000, arg = "A______wwww\235\235\235_N____" of length 020.

A particularly malicious attacker could craft an executable that executes when *analysed* by objdump, nm or gdb, or any other libbfd / libiberty - based forensics tool (if the demangling option is switched on).

--
Marcel Böhme
Postdoctoral Researcher
TSUNAMi Security Research Lab
National University of Singapore
Comment 5 Marcel Böhme 2016-02-06 17:20:09 UTC
Created attachment 37612 [details]
Debug This
Comment 6 Manuel López-Ibáñez 2016-02-07 01:33:02 UTC
(In reply to Marcel Böhme from comment #5)
> Created attachment 37612 [details]
> Debug This

Scary.
Comment 7 Marcel Böhme 2016-02-07 07:04:37 UTC
Created attachment 37620 [details]
Valgrind This

$ cat compileme.c
#include<stdio.h>
#include<stdlib.h>

const char* ____________________X00020A___R0020A__U000R03000N99999999_020A__K000(){
  char *p;
  p = (char *) malloc(19);
  p = (char *) malloc(12);
  free(p);
  p = (char *) malloc(16);
  return "Hello World!";
}

int main()
{
   printf("%s\n",____________________X00020A___R0020A__U000R03000N99999999_020A__K000());
   return 0;
}

$ g++ compileme.c -o temp
$ sed -b s/Z68/_20/g temp > valgrindme
$ chmod u+x valgrindme
$ ./valgrindme
Hello World!
$ valgrind --leak-check=yes ./valgrindme
..
$ gdb ./valgrindme
..
Comment 9 Marcel Böhme 2016-03-02 08:02:06 UTC
Created attachment 37839 [details]
Proposed Patch

* Limiting the length of the mangled string to 264k characters.
* Limiting the loop iterations to 256 (max. of C++ function parameters).
Comment 10 Markus Trippelsdorf 2016-03-02 08:22:55 UTC
Patches should be posted to: gcc-patches@gcc.gnu.org
Comment 11 Manuel López-Ibáñez 2016-03-02 20:45:16 UTC
The policy of GNU software is to avoid arbitrary implementation limits whenever possible.

(In reply to Marcel Böhme from comment #4)
> with n=2*(length of decl + length of arg) characters. Since n is a signed
> int, n wraps over at some iteration. Since, realloc expects n to be
> unsigned, we end up allocating less memory then actually needed. In the

Why n is signed if realloc expects it to be unsigned? Aren't the lengths measured in size_t also? Moreover, it should be trivial to check for overflow before computing n (both from the sum and from *2) by using SIZE_MAX. It should fail only if any intermediate step overflows size_t.
Comment 12 Bernd Schmidt 2016-04-08 12:10:53 UTC
Author: bernds
Date: Fri Apr  8 12:10:21 2016
New Revision: 234829

URL: https://gcc.gnu.org/viewcvs?rev=234829&root=gcc&view=rev
Log:
Fix memory allocation size overflows (PR69687, patch by Marcel Böhme)

	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.


Modified:
    trunk/libiberty/ChangeLog
    trunk/libiberty/cplus-dem.c
Comment 13 Bernd Schmidt 2016-04-08 12:27:35 UTC
Fixed.
Comment 14 Jakub Jelinek 2016-05-19 10:45:03 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 15 Jakub Jelinek 2016-05-19 12:06:13 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