-
Notifications
You must be signed in to change notification settings - Fork 563
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
4 changed files
with
527 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
From 033bc52bb6190393c8eed80925fa78cc35b40c6d Mon Sep 17 00:00:00 2001 | ||
From: Tom Tromey <[email protected]> | ||
Date: Wed, 16 Aug 2023 11:29:19 -0600 | ||
Subject: [PATCH] Avoid buffer overflow in ada_decode | ||
|
||
A bug report pointed out a buffer overflow in ada_decode, which Keith | ||
helpfully analyzed. ada_decode had a logic error when the input was | ||
all digits. While this isn't valid -- and would probably only appear | ||
in fuzzer tests -- it still should be handled properly. | ||
|
||
This patch adds a missing bounds check. Tested with the self-tests in | ||
an asan build. | ||
|
||
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30639 | ||
Reviewed-by: Keith Seitz <[email protected]> | ||
--- | ||
gdb/ada-lang.c | 19 ++++++++++++++++++- | ||
1 file changed, 18 insertions(+), 1 deletion(-) | ||
|
||
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c | ||
index 4a9a6e0f38f..2f934b1e79a 100644 | ||
--- a/gdb/ada-lang.c | ||
+++ b/gdb/ada-lang.c | ||
@@ -57,6 +57,7 @@ | ||
#include "cli/cli-utils.h" | ||
#include "gdbsupport/function-view.h" | ||
#include "gdbsupport/byte-vector.h" | ||
+#include "gdbsupport/selftest.h" | ||
#include <algorithm> | ||
#include "ada-exp.h" | ||
#include "charset.h" | ||
@@ -1377,7 +1378,7 @@ ada_decode (const char *encoded, bool wrap, bool operators) | ||
i -= 1; | ||
if (i > 1 && encoded[i] == '_' && encoded[i - 1] == '_') | ||
len0 = i - 1; | ||
- else if (encoded[i] == '$') | ||
+ else if (i >= 0 && encoded[i] == '$') | ||
len0 = i; | ||
} | ||
|
||
@@ -1574,6 +1575,18 @@ ada_decode (const char *encoded, bool wrap, bool operators) | ||
return decoded; | ||
} | ||
|
||
+#ifdef GDB_SELF_TEST | ||
+ | ||
+static void | ||
+ada_decode_tests () | ||
+{ | ||
+ /* This isn't valid, but used to cause a crash. PR gdb/30639. The | ||
+ result does not really matter very much. */ | ||
+ SELF_CHECK (ada_decode ("44") == "44"); | ||
+} | ||
+ | ||
+#endif | ||
+ | ||
/* Table for keeping permanent unique copies of decoded names. Once | ||
allocated, names in this table are never released. While this is a | ||
storage leak, it should not be significant unless there are massive | ||
@@ -13984,4 +13997,8 @@ DWARF attribute."), | ||
gdb::observers::new_objfile.attach (ada_new_objfile_observer, "ada-lang"); | ||
gdb::observers::free_objfile.attach (ada_free_objfile_observer, "ada-lang"); | ||
gdb::observers::inferior_exit.attach (ada_inferior_exit, "ada-lang"); | ||
+ | ||
+#ifdef GDB_SELF_TEST | ||
+ selftests::register_test ("ada-decode", ada_decode_tests); | ||
+#endif | ||
} | ||
-- | ||
2.43.5 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
From 58abdf887821a5da09ba184c6e400a3bc5cccd5a Mon Sep 17 00:00:00 2001 | ||
From: Keith Seitz <[email protected]> | ||
Date: Wed, 2 Aug 2023 08:35:11 -0700 | ||
Subject: [PATCH] Verify COFF symbol stringtab offset | ||
|
||
This patch addresses an issue with malformed/fuzzed debug information that | ||
was recently reported in gdb/30639. That bug specifically deals with | ||
an ASAN issue, but the reproducer provided by the reporter causes a | ||
another failure outside of ASAN: | ||
|
||
$ ./gdb --data-directory data-directory -nx -q UAF_2 | ||
Reading symbols from /home/keiths/UAF_2... | ||
|
||
|
||
Fatal signal: Segmentation fault | ||
----- Backtrace ----- | ||
0x59a53a gdb_internal_backtrace_1 | ||
../../src/gdb/bt-utils.c:122 | ||
0x59a5dd _Z22gdb_internal_backtracev | ||
../../src/gdb/bt-utils.c:168 | ||
0x786380 handle_fatal_signal | ||
../../src/gdb/event-top.c:889 | ||
0x7864ec handle_sigsegv | ||
../../src/gdb/event-top.c:962 | ||
0x7ff354c5fb6f ??? | ||
0x611f9a process_coff_symbol | ||
../../src/gdb/coffread.c:1556 | ||
0x611025 coff_symtab_read | ||
../../src/gdb/coffread.c:1172 | ||
0x60f8ff coff_read_minsyms | ||
../../src/gdb/coffread.c:549 | ||
0x60fe4b coff_symfile_read | ||
../../src/gdb/coffread.c:698 | ||
0xbde0f6 read_symbols | ||
../../src/gdb/symfile.c:772 | ||
0xbde7a3 syms_from_objfile_1 | ||
../../src/gdb/symfile.c:966 | ||
0xbde867 syms_from_objfile | ||
../../src/gdb/symfile.c:983 | ||
0xbded42 symbol_file_add_with_addrs | ||
../../src/gdb/symfile.c:1086 | ||
0xbdf083 _Z24symbol_file_add_from_bfdRKN3gdb7ref_ptrI3bfd18gdb_bfd_ref_policyEEPKc10enum_flagsI16symfile_add_flagEPSt6vectorI14other_sectionsSaISC_EES8_I12objfile_flagEP7objfile | ||
../../src/gdb/symfile.c:1166 | ||
0xbdf0d2 _Z15symbol_file_addPKc10enum_flagsI16symfile_add_flagEPSt6vectorI14other_sectionsSaIS5_EES1_I12objfile_flagE | ||
../../src/gdb/symfile.c:1179 | ||
0xbdf197 symbol_file_add_main_1 | ||
../../src/gdb/symfile.c:1203 | ||
0xbdf13e _Z20symbol_file_add_mainPKc10enum_flagsI16symfile_add_flagE | ||
../../src/gdb/symfile.c:1194 | ||
0x90f97f symbol_file_add_main_adapter | ||
../../src/gdb/main.c:549 | ||
0x90f895 catch_command_errors | ||
../../src/gdb/main.c:518 | ||
0x9109b6 captured_main_1 | ||
../../src/gdb/main.c:1203 | ||
0x910fc8 captured_main | ||
../../src/gdb/main.c:1310 | ||
0x911067 _Z8gdb_mainP18captured_main_args | ||
../../src/gdb/main.c:1339 | ||
0x418c71 main | ||
../../src/gdb/gdb.c:39 | ||
--------------------- | ||
A fatal error internal to GDB has been detected, further | ||
debugging is not possible. GDB will now terminate. | ||
|
||
This is a bug, please report it. For instructions, see: | ||
<https://www.gnu.org/software/gdb/bugs/>. | ||
|
||
Segmentation fault (core dumped) | ||
|
||
The issue here is that the COFF offset for the fuzzed symbol's | ||
name is outside the string table. That is, the offset is greater | ||
than the actual string table size. | ||
|
||
coffread.c:getsymname actually contains a FIXME about this, and that's | ||
what I've chosen to address to fix this issue, following what is done | ||
in the DWARF reader: | ||
|
||
$ ./gdb --data-directory data-directory -nx -q UAF_2 | ||
Reading symbols from /home/keiths/UAF_2... | ||
COFF Error: string table offset (256) outside string table (length 0) | ||
(gdb) | ||
|
||
Unfortunately, I haven't any idea how else to test this patch since | ||
COFF is not very common anymore. GCC removed support for it five | ||
years ago with GCC 8. | ||
--- | ||
gdb/coffread.c | 7 +++++-- | ||
1 file changed, 5 insertions(+), 2 deletions(-) | ||
|
||
diff --git a/gdb/coffread.c b/gdb/coffread.c | ||
index f8e14d8ad93..ae7632d49cb 100644 | ||
--- a/gdb/coffread.c | ||
+++ b/gdb/coffread.c | ||
@@ -159,6 +159,7 @@ static file_ptr linetab_offset; | ||
static file_ptr linetab_size; | ||
|
||
static char *stringtab = NULL; | ||
+static long stringtab_length = 0; | ||
|
||
extern void stabsread_clear_cache (void); | ||
|
||
@@ -1303,6 +1304,7 @@ init_stringtab (bfd *abfd, file_ptr offset, gdb::unique_xmalloc_ptr<char> *stora | ||
/* This is in target format (probably not very useful, and not | ||
currently used), not host format. */ | ||
memcpy (stringtab, lengthbuf, sizeof lengthbuf); | ||
+ stringtab_length = length; | ||
if (length == sizeof length) /* Empty table -- just the count. */ | ||
return 0; | ||
|
||
@@ -1322,8 +1324,9 @@ getsymname (struct internal_syment *symbol_entry) | ||
|
||
if (symbol_entry->_n._n_n._n_zeroes == 0) | ||
{ | ||
- /* FIXME: Probably should be detecting corrupt symbol files by | ||
- seeing whether offset points to within the stringtab. */ | ||
+ if (symbol_entry->_n._n_n._n_offset > stringtab_length) | ||
+ error (_("COFF Error: string table offset (%ld) outside string table (length %ld)"), | ||
+ symbol_entry->_n._n_n._n_offset, stringtab_length); | ||
result = stringtab + symbol_entry->_n._n_n._n_offset; | ||
} | ||
else | ||
-- | ||
2.43.5 |
Oops, something went wrong.