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
Created attachment 37593 [details] Test Case #2
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.
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
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
Created attachment 37612 [details] Debug This
(In reply to Marcel Böhme from comment #5) > Created attachment 37612 [details] > Debug This Scary.
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 ..
Downstream: * Valgrind: https://bugs.kde.org/show_bug.cgi?id=359181 * GDB: https://sourceware.org/bugzilla/show_bug.cgi?id=19597 * Binutils: https://sourceware.org/bugzilla/show_bug.cgi?id=19571
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).
Patches should be posted to: gcc-patches@gcc.gnu.org
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.
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
Fixed.
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
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