gdb: bfd_get_symbol_leading_char vs. ""

gdb: bfd_get_symbol_leading_char vs. ""

[Alan Modra]

Some places matching the first char of a string against
bfd_get_symbol_leading_char, which may be zero, didn't check for "".
This could lead to accesses past the end of the string and potential
buffer overruns.  Fix that, and also get rid of a stupid optimisation
in dbxread when looking for "__DYNAMIC" that also might access past
the end of a string.

diff --git a/gdb/coffread.c b/gdb/coffread.c
index 13610998ad7..7998327fdaf 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -64,8 +64,8 @@ static const registry<objfile>::key<coff_symfile_info> coff_objfile_data_key;
 
 /* Translate an external name string into a user-visible name.  */
 #define	EXTERNAL_NAME(string, abfd) \
-	(string[0] == bfd_get_symbol_leading_char (abfd) \
-	? string + 1 : string)
+  (*string && *string == bfd_get_symbol_leading_char (abfd)	\
+   ? string + 1 : string)
 
 /* To be an sdb debug type, type must have at least a basic or primary
    derived type.  Using this rather than checking against T_NULL is
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 7cddf6586ed..4c585efd192 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -479,14 +479,15 @@ record_minimal_symbol (minimal_symbol_reader &reader,
 	 Record it as global even if it's local, not global, so
 	 lookup_minimal_symbol can find it.  We don't check symbol_leading_char
 	 because for SunOS4 it always is '_'.  */
-      if (name[8] == 'C' && strcmp ("__DYNAMIC", name) == 0)
+      if (strcmp ("__DYNAMIC", name) == 0)
 	ms_type = mst_data;
 
       /* Same with virtual function tables, both global and static.  */
       {
 	const char *tempstring = name;
 
-	if (tempstring[0] == bfd_get_symbol_leading_char (objfile->obfd.get ()))
+	if (*tempstring
+	    && *tempstring == bfd_get_symbol_leading_char (objfile->obfd.get ()))
 	  ++tempstring;
 	if (is_vtable_name (tempstring))
 	  ms_type = mst_data;
@@ -2254,7 +2255,8 @@ read_ofile_symtab (struct objfile *objfile, legacy_psymtab *pst)
 	    processing_gcc_compilation = 1;
 	  else if (strcmp (namestring, GCC2_COMPILED_FLAG_SYMBOL) == 0)
 	    processing_gcc_compilation = 2;
-	  if (tempstring[0] == bfd_get_symbol_leading_char (symfile_bfd))
+	  if (*tempstring
+	      && *tempstring == bfd_get_symbol_leading_char (symfile_bfd))
 	    ++tempstring;
 	  if (startswith (tempstring, "__gnu_compiled"))
 	    processing_gcc_compilation = 2;
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 5154d1a31a3..615e08c6afa 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -394,7 +394,8 @@ macho_resolve_oso_sym_with_minsym (struct objfile *main_objfile, asymbol *sym)
   struct bound_minimal_symbol msym;
   const char *name = sym->name;
 
-  if (name[0] == bfd_get_symbol_leading_char (main_objfile->obfd.get ()))
+  if (*name
+      && *name == bfd_get_symbol_leading_char (main_objfile->obfd.get ()))
     ++name;
   msym = lookup_minimal_symbol (name, NULL, main_objfile);
   if (msym.minsym == NULL)

-- 
Alan Modra
Australia Development Lab, IBM

Re: gdb: bfd_get_symbol_leading_char vs. ""

[Tom Tromey]

>>>>> "Alan" == Alan Modra via Gdb-patches <gdb-patches@sourceware.org> writes:

Alan> Some places matching the first char of a string against
Alan> bfd_get_symbol_leading_char, which may be zero, didn't check for "".
Alan> This could lead to accesses past the end of the string and potential
Alan> buffer overruns.

Thank you.

Alan> Fix that, and also get rid of a stupid optimisation
Alan> in dbxread when looking for "__DYNAMIC" that also might access past
Alan> the end of a string.

Gross stuff even by old school standards.

Anyway, this is ok.

Tom