readelf looping in process_archive

Message ID 20200325120212.GG4583@bubble.grove.modra.org
State New
Headers show
Series
  • readelf looping in process_archive
Related show

Commit Message

Stefan Schulze Frielinghaus via Binutils March 25, 2020, 12:02 p.m.
With a crafted "negative" ar_hdr.ar_size it is possible to make
readelf loop.  This patch catches the overflow in a file offset
calculation.

	* readelf.c (process_archive): Prevent endless loop.


-- 
Alan Modra
Australia Development Lab, IBM

Comments

Stefan Schulze Frielinghaus via Binutils March 27, 2020, 12:13 a.m. | #1
This patch fixes a leak of qualified_name caused by 4c83662712 and a
double free introduced by fd486f32d1.  Not breaking out of the loop
results in an error: "failed to seek to next archive header".  That's
slightly better than silently preventing the possibility of endless
loops.

	* readelf.c (process_archive): Don't double free qualified_name.
	Don't break out of loop with "negative" archive_file_size, just
	set file offset to max.

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 9bc15e4d0b..eb41e10dae 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -20461,7 +20461,6 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
 
 	  close_file (member_filedata);
 	  free (member_file_name);
-	  free (qualified_name);
 	}
       else if (is_thin_archive)
 	{
@@ -20511,7 +20510,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
 	  arch.next_arhdr_offset += archive_file_size;
 	  /* Stop looping with "negative" archive_file_size.  */
 	  if (arch.next_arhdr_offset < archive_file_size)
-	    break;
+	    arch.next_arhdr_offset = -1ul;
 	}
 
       free (qualified_name);

-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 1f0f49222f..9bc15e4d0b 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -20505,11 +20505,13 @@  process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
 	{
 	  free (name);
 	  archive_file_offset = arch.next_arhdr_offset;
-	  arch.next_arhdr_offset += archive_file_size;
-
 	  filedata->file_name = qualified_name;
 	  if (! process_object (filedata))
 	    ret = FALSE;
+	  arch.next_arhdr_offset += archive_file_size;
+	  /* Stop looping with "negative" archive_file_size.  */
+	  if (arch.next_arhdr_offset < archive_file_size)
+	    break;
 	}
 
       free (qualified_name);