microsoft · miz060 · Oct 10, 2024
@@ -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
@@ -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