[PATCH] Fix a buffer overflow in libgfortran

[Jakub Jelinek]

On Mon, Apr 07, 2008 at 09:11:23AM -0400, Jakub Jelinek wrote:
> Still, even with this patch I fear e.g. that nl->var_name doesn't have any
> upper bound, so at least wherever you put that snprintf should be used.

Apparently the frontend limits all symbols to 63 chars, still libgfortran
shouldn't assume it silently, unless it verifies that.  Even then 100 bytes
for e.g. up to 36 chars message from nml_parse_identifier + strlen
(nl->var_name) (up to 63) + 24 is too short.  Testcase that still overflows
the stack is e.g.:

! { dg-do run }

module global
  type             ::  mt
    character(len=2) ::  c012345678901234567890123456789012345678901234567890123456789h(2) = (/"aa","bb"/)
  end type mt
  type             ::  bt
    integer        ::  i(2) = (/1,2/)
    type(mt)       ::  m(2)
  end type bt
end module global

program namelist_47
  use global
  type(bt)         ::  x(2)
  character(140)    ::  teststring 
  namelist /mynml/ x

  teststring = " x(2)%m%c012345678901234567890123456789012345678901234567890123456789h(:)(2:2) = 'z','z',"
  call writenml (teststring)
  teststring = " x(2)%m(2)%c012345678901234567890123456789012345678901234567890123456789h(:)(2) = 'z','z',"
  call writenml (teststring)
  teststring = " x(2)%m(2)%c012345678901234567890123456789012345678901234567890123456789h(:)(:3) = 'z','z',"
  call writenml (teststring)
  teststring = " x(2)%m(2)%c012345678901234567890123456789012345678901234567890123456789h(1:2)(k:) = 'z','z',"
  call writenml (teststring)
  
contains

subroutine writenml (astring)
  character(140), intent(in)  :: astring
  character(300)   :: errmessage
  integer          :: ierror

  open (10, status="scratch", delim='apostrophe')
  write (10, '(A)') "&MYNML"
  write (10, '(A)') astring
  write (10, '(A)') "/"
  rewind (10)
  read (10, nml = mynml, iostat=ierror, iomsg=errmessage)
  if (ierror == 0) call abort
  print '(a)', trim(errmessage)
  close (10)

end subroutine writenml

end program namelist_47
! { dg-final { cleanup-modules "global" } }

So, snprintf should be used plus the nml_err_msg size bumped to
at least 150 on branches where generate_error/internal_error isn't
changed into vararg function.

OT, while looking at list_read.c, I've noticed inefficiencies, e.g.
I don't see any point to allocate memory in nml_touch_nodes and
nml_read_obj, all the following code then does is
strncmp (something, orig_nl->var_name with "%" appended to it, ...)
That can very well be done e.g. in nml_touch_nodes with
--- list_read.c 2008-04-07 15:48:44.000000000 +0200
+++ list_read.c    2008-04-07 16:06:35.000000000 +0200
@@ -2136,14 +2136,13 @@ find_nml_node (st_parameter_dt *dtp, cha
 static void
 nml_touch_nodes (namelist_info * nl)
 {
-  index_type len = strlen (nl->var_name) + 1;
+  char * ext_name = nl->var_name;
+  index_type len = strlen (ext_name);
   int dim;
-  char * ext_name = (char*)get_mem (len + 1);
-  memcpy (ext_name, nl->var_name, len-1);
-  memcpy (ext_name + len - 1, "%", 2);
   for (nl = nl->next; nl; nl = nl->next)
     {
-      if (strncmp (nl->var_name, ext_name, len) == 0)
+      if (strncmp (nl->var_name, ext_name, len) == 0
+         && nl->var_name[len] == '%')
        {
          nl->touched = 1;
          for (dim=0; dim < nl->var_rank; dim++)
@@ -2157,7 +2156,6 @@ nml_touch_nodes (namelist_info * nl)
       else
        break;
     }
-  free_mem (ext_name);
   return;
 }
 

	Jakub