Hi,

while working on free software ports to Windows CE I noticed two bugs in
binutils' BFD support for some PE files (for example, kmail-mobile.exe built
with MSVC).  Fixes for both are included below.  Copyright assignments by g10
Code GmbH are on file at the FSF.  If you need anything else, just let me
know.  The file that triggers these bugs can be found at:

ftp://ftp.g10code.com/people/marcus/kmail-mobile-binutils-test.exe.gz

The first issue concerns import tables where the thunk table is found in a
different section.  In this case, BFD tries to load DATASIZE bytes from that
section at the beginning of the thunk array, but DATASIZE is the remaining
bytes in the import table section starting from the beginning of the import
table.  This number is in no way related to the size of the thunk table or the
section in which this thunk table is to be found.  So, my patch introduces a
new size, limit_size, which is correctly calculated and used in the
appropriate places.  Without the patch, no import symbols would be shown for
kmail-mobile.exe (Visual Studio 2008, Windows CE), because the data section is
in this case not large enough to read DATASIZE bytes from it.  With the patch,
loading LIMIT_SIZE bytes succeeds and all import symbols are shown correctly.

The second issue concerns the support for compressed pdata support for Windows
CE.  In this code is a simple memory leak.  First, the whole section is
malloced and copied to TDATA, then immediately TDATA is overwritten with a
much smaller buffer to which only the required section data is copied, leaking
memory in the size of the section for each entry in the table.  For
kmail-mobile.exe, the table is very large (hundreds of entries), leaking
Gigabytes of memory quickly and basically creating denial of service attack.

Thanks a lot,
Marcus Brinkmann
mar...@g10code.com

--
g10 Code GmbH       http://g10code.com      AmtsGer. Wuppertal HRB 14459
Hüttenstr. 61                               Geschäftsführung Werner Koch
D-40699 Erkrath  -=- The GnuPG Experts -=-  USt-Id DE215605608
2010-09-03  Marcus Brinkmann  <mar...@g10code.de>

	* peXXigen.c (pe_print_idata): Use correct size limit to load
          thunk table from different section.

Index: peXXigen.c
===================================================================
RCS file: /cvs/src/src/bfd/peXXigen.c,v
retrieving revision 1.64
diff -u -p -r1.64 peXXigen.c
--- peXXigen.c	27 Jun 2010 04:07:53 -0000	1.64
+++ peXXigen.c	2 Sep 2010 23:02:08 -0000
@@ -1226,6 +1226,7 @@ pe_print_idata (bfd * abfd, void * vfile
 	  bfd_size_type ft_datasize;
 	  int ft_idx;
 	  int ft_allocated = 0;
+	  bfd_size_type limit_size;
 
 	  fprintf (file, _("\tvma:  Hint/Ord Member-Name Bound-To\n"));
 
@@ -1262,17 +1263,19 @@ pe_print_idata (bfd * abfd, void * vfile
 		{
 		  ft_data = data;
 		  ft_idx = first_thunk - adj;
+		  limit_size = section->size - ft_idx;
 		}
 	      else
 		{
 		  ft_idx = first_thunk - (ft_section->vma - extra->ImageBase);
-		  ft_data = (bfd_byte *) bfd_malloc (datasize);
+		  limit_size = ft_section->size - ft_idx;
+		  ft_data = (bfd_byte *) bfd_malloc (limit_size);
 		  if (ft_data == NULL)
 		    continue;
 
-		  /* Read datasize bfd_bytes starting at offset ft_idx.  */
+		  /* Read limit_size bfd_bytes starting at offset ft_idx.  */
 		  if (! bfd_get_section_contents
-		      (abfd, ft_section, ft_data, (bfd_vma) ft_idx, datasize))
+		      (abfd, ft_section, ft_data, (bfd_vma) ft_idx, limit_size))
 		    {
 		      free (ft_data);
 		      continue;
@@ -1285,7 +1288,7 @@ pe_print_idata (bfd * abfd, void * vfile
 
 	  /* Print HintName vector entries.  */
 #ifdef COFF_WITH_pex64
-	  for (j = 0; j < datasize; j += 8)
+	  for (j = 0; j < limit_size; j += 8)
 	    {
 	      unsigned long member = bfd_get_32 (abfd, data + idx + j);
 	      unsigned long member_high = bfd_get_32 (abfd, data + idx + j + 4);
@@ -1316,7 +1319,7 @@ pe_print_idata (bfd * abfd, void * vfile
 	      fprintf (file, "\n");
 	    }
 #else
-	  for (j = 0; j < datasize; j += 4)
+	  for (j = 0; j < limit_size; j += 4)
 	    {
 	      unsigned long member = bfd_get_32 (abfd, data + idx + j);
 
2010-09-03  Marcus Brinkmann  <mar...@g10code.de>

	* peXXigen.c (_bfd_XX_print_ce_compressed_pdata): Fix memory leak.

Index: peXXigen.c
===================================================================
RCS file: /cvs/src/src/bfd/peXXigen.c,v
retrieving revision 1.64
diff -u -p -r1.64 peXXigen.c
--- peXXigen.c	27 Jun 2010 04:07:53 -0000	1.64
+++ peXXigen.c	2 Sep 2010 23:02:08 -0000
@@ -1828,7 +1831,6 @@ _bfd_XX_print_ce_compressed_pdata (bfd *
       bfd_vma other_data;
       bfd_vma prolog_length, function_length;
       int flag32bit, exception_flag;
-      bfd_byte *tdata = 0;
       asection *tsection;
 
       if (i + PDATA_ROW_SIZE > stop)
@@ -1860,36 +1862,29 @@ _bfd_XX_print_ce_compressed_pdata (bfd *
       if (tsection && coff_section_data (abfd, tsection)
 	  && pei_section_data (abfd, tsection))
 	{
-	  if (bfd_malloc_and_get_section (abfd, tsection, & tdata))
-	    {
-	      int xx = (begin_addr - 8) - tsection->vma;
+	  int xx = (begin_addr - 8) - tsection->vma;
+	  bfd_byte *tdata = 0;
 
-	      tdata = (bfd_byte *) bfd_malloc (8);
-	      if (bfd_get_section_contents (abfd, tsection, tdata, (bfd_vma) xx, 8))
+	  tdata = (bfd_byte *) bfd_malloc (8);
+	  if (tdata && bfd_get_section_contents (abfd, tsection, tdata, (bfd_vma) xx, 8))
+	    {
+	      bfd_vma eh, eh_data;
+	      
+	      eh = bfd_get_32 (abfd, tdata);
+	      eh_data = bfd_get_32 (abfd, tdata + 4);
+	      fprintf (file, "%08x  ", (unsigned int) eh);
+	      fprintf (file, "%08x", (unsigned int) eh_data);
+	      if (eh != 0)
 		{
-		  bfd_vma eh, eh_data;
-
-		  eh = bfd_get_32 (abfd, tdata);
-		  eh_data = bfd_get_32 (abfd, tdata + 4);
-		  fprintf (file, "%08x  ", (unsigned int) eh);
-		  fprintf (file, "%08x", (unsigned int) eh_data);
-		  if (eh != 0)
-		    {
-		      const char *s = my_symbol_for_address (abfd, eh, &cache);
-
-		      if (s)
-			fprintf (file, " (%s) ", s);
-		    }
+		  const char *s = my_symbol_for_address (abfd, eh, &sym_cache);
+		  
+		  if (s)
+		    fprintf (file, " (%s) ", s);
 		}
 	      free (tdata);
 	    }
-	  else
-	    {
-	      if (tdata)
-		free (tdata);
-	    }
 	}
-
+      
       fprintf (file, "\n");
     }
 
_______________________________________________
bug-binutils mailing list
bug-binutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-binutils

Reply via email to