[PATCH] readelf: handle corrupted chains better

[PATCH] readelf: handle corrupted chains better

[Mike Frysinger]

The current chain walker tries to protect itself against loops, by only
works with loops of length 1: a chain that points to itself.  If you have
a chain longer than that (3->4->3->4->...), readelf will still hang.

Since we know the max length of the chain, simply abort when we've walked
more times than that.  The only way that could have happened is if there
was a loop.

2015-02-24  Mike Frysinger  <vapier@gentoo.org>

	PR binutils/17531
	* readelf.c (process_symbol_table): Declare chained.  Increment it
	in every loop.  Abort when chained is larger than nchains.  Move
	error check outside of chain loop.
---
 binutils/readelf.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 66b8f98..f1322d3 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -10722,6 +10722,7 @@ process_symbol_table (FILE * file)
       unsigned long maxlength = 0;
       unsigned long nzero_counts = 0;
       unsigned long nsyms = 0;
+      unsigned long chained;
 
       printf (_("\nHistogram for bucket list length (total of %lu buckets):\n"),
 	      (unsigned long) nbuckets);
@@ -10736,21 +10737,23 @@ process_symbol_table (FILE * file)
       printf (_(" Length  Number     %% of total  Coverage\n"));
       for (hn = 0; hn < nbuckets; ++hn)
 	{
-	  for (si = buckets[hn]; si > 0 && si < nchains && si < nbuckets; si = chains[si])
+	  for (si = buckets[hn], chained = 0;
+	       si > 0 && si < nchains && si < nbuckets && chained <= nchains;
+	       si = chains[si], ++chained)
 	    {
 	      ++nsyms;
 	      if (maxlength < ++lengths[hn])
 		++maxlength;
-
-	      /* PR binutils/17531: A corrupt binary could contain broken
-		 histogram data.  Do not go into an infinite loop trying
-		 to process it.  */
-	      if (chains[si] == si)
-		{
-		  error (_("histogram chain links to itself\n"));
-		  break;
-		}
 	    }
+
+	    /* PR binutils/17531: A corrupt binary could contain broken
+	       histogram data.  Do not go into an infinite loop trying
+	       to process it.  */
+	    if (chained > nchains)
+	      {
+		error (_("histogram chain is corrupt\n"));
+		break;
+	      }
 	}
 
       counts = (unsigned long *) calloc (maxlength + 1, sizeof (*counts));
-- 
2.3.0

Re: [PATCH] readelf: handle corrupted chains better

[Nicholas Clifton]

Hi Mike,

> 2015-02-24  Mike Frysinger  <vapier@gentoo.org>
>
> 	PR binutils/17531
> 	* readelf.c (process_symbol_table): Declare chained.  Increment it
> 	in every loop.  Abort when chained is larger than nchains.  Move
> 	error check outside of chain loop.

Approved - please apply.

(Thanks for doing this.  I knew that there was a better way to protect 
against loops, but my brain just could not come up with it).

Cheers
   Nick

Re: [PATCH] readelf: handle corrupted chains better

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On 24 Feb 2015 09:56, Nicholas Clifton wrote:
> > 2015-02-24  Mike Frysinger  <vapier@gentoo.org>
> >
> > 	PR binutils/17531
> > 	* readelf.c (process_symbol_table): Declare chained.  Increment it
> > 	in every loop.  Abort when chained is larger than nchains.  Move
> > 	error check outside of chain loop.
> 
> Approved - please apply.

done

> (Thanks for doing this.  I knew that there was a better way to protect 
> against loops, but my brain just could not come up with it).

no worries ... i hit the same bug in a package i maintain so i figured i'd fix 
readelf too while i was at it
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]