PR fortran/95530, PR fortran/95537 - Buffer overflows with long symbols

Message ID trinity-d039c8fb-16e5-474f-95e4-2b90a462b1b7-1591375540502@3c-app-gmx-bap51
State New
Headers show
Series
  • PR fortran/95530, PR fortran/95537 - Buffer overflows with long symbols
Related show

Commit Message

Harald Anlauf June 5, 2020, 4:45 p.m.
The testcases for fixes of buffer overflows did show some fallout in
the testsuite that occurred only with suitably instrumented compilers.
Bill Seurer provided useful backtraces on powerpc64 that helped to nail
down the issues.  To verifiy that the newly chosen buffer size is now
sufficent, a check is put into place.

Regtested on x86_64-pc-linux-gnu, and confirmed in the PRs by Bill.

OK for master?

In case the fixes for PR95090 or PR95106 are backported, this one will be
needed, too.

Thanks,
Harald


PR fortran/95530, PR fortran/95537 - Buffer overflows with long symbols

The testcases for PR95090 and PR95106 trigger buffer overflows with long
symbols that were found with an instrumented compiler.  Enlarge the
affected buffers, and add checks that the buffers will suffice.

2020-06-05  Harald Anlauf  <anlauf@gmx.de>

gcc/fortran/
	PR fortran/95530
	PR fortran/95537
	* decl.c (gfc_match_decl_type_spec): Enlarge buffer, and enhance
	string copy to detect buffer overflow.
	* gfortran.h (gfc_common_head): Enlarge buffer.
	* trans-common.c (finish_equivalences): Enhance string copy to
	detect buffer overflow.

Comments

Am 05.06.20 um 18:45 schrieb Harald Anlauf:
> The testcases for fixes of buffer overflows did show some fallout in

> the testsuite that occurred only with suitably instrumented compilers.

> Bill Seurer provided useful backtraces on powerpc64 that helped to nail

> down the issues.  To verifiy that the newly chosen buffer size is now

> sufficent, a check is put into place.

> 

> Regtested on x86_64-pc-linux-gnu, and confirmed in the PRs by Bill.

> 

> OK for master?


OK.

> In case the fixes for PR95090 or PR95106 are backported, this one will be

> needed, too.


They are safe enough for a backport, so if you want to, please feel
free.

Thanks for the patch!

Regards

	Thomas

Patch

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3ad5559c3ec..1c1626d3fa4 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -4094,7 +4094,8 @@  match_byte_typespec (gfc_typespec *ts)
 match
 gfc_match_decl_type_spec (gfc_typespec *ts, int implicit_flag)
 {
-  char name[GFC_MAX_SYMBOL_LEN + 1];
+  /* Provide sufficient space to hold "pdtsymbol".  */
+  char name[GFC_MAX_SYMBOL_LEN + 1 + 3];
   gfc_symbol *sym, *dt_sym;
   match m;
   char c;
@@ -4284,7 +4285,11 @@  gfc_match_decl_type_spec (gfc_typespec *ts, int implicit_flag)
 	    return m;
 	  gcc_assert (!sym->attr.pdt_template && sym->attr.pdt_type);
 	  ts->u.derived = sym;
-	  strcpy (name, gfc_dt_lower_string (sym->name));
+	  const char* lower = gfc_dt_lower_string (sym->name);
+	  size_t len = strnlen (lower, sizeof (name));
+	  gcc_assert (len < sizeof (name));
+	  memcpy (name, lower, len);
+	  name[len] = '\0';
 	}

       if (sym && sym->attr.flavor == FL_STRUCT)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 5af44847f9b..0ef7b1b0eff 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1677,7 +1677,8 @@  typedef struct gfc_common_head
   char use_assoc, saved, threadprivate;
   unsigned char omp_declare_target : 1;
   unsigned char omp_declare_target_link : 1;
-  char name[GFC_MAX_SYMBOL_LEN + 1];
+  /* Provide sufficient space to hold "symbol.eq.1234567890".  */
+  char name[GFC_MAX_SYMBOL_LEN + 1 + 14];
   struct gfc_symbol *head;
   const char* binding_label;
   int is_bind_c;
diff --git a/gcc/fortran/trans-common.c b/gcc/fortran/trans-common.c
index 3775a8bea74..1acc336eacf 100644
--- a/gcc/fortran/trans-common.c
+++ b/gcc/fortran/trans-common.c
@@ -1314,7 +1314,11 @@  finish_equivalences (gfc_namespace *ns)
 	      c->where = ns->proc_name->declared_at;
 	    else if (ns->is_block_data)
 	      c->where = ns->sym_root->n.sym->declared_at;
-	    strcpy (c->name, z->module);
+
+	    size_t len = strlen (z->module);
+	    gcc_assert (len < sizeof (c->name));
+	    memcpy (c->name, z->module, len);
+	    c->name[len] = '\0';
 	  }
 	else
 	  c = NULL;