[5/8] libctf: don't dereference out-of-bounds locations in the qualifier hashtab

Message ID 20210324012158.35472-5-nick.alcock@oracle.com
State New
Headers show
Series
  • [1/8] libctf, dump: do not emit size or alignment if it would error
Related show

Commit Message

Alan Modra via Binutils March 24, 2021, 1:21 a.m.
isqualifier, which is used by ctf_lookup_by_name to figure out if a
given word in a type name is a qualifier, takes the address of a
possibly out-of-bounds location before checking its bounds.

In any reasonable compiler this will just lead to a harmless address
computation that is then discarded if out-of-bounds, but it's still
undefined behaviour and the sanitizer rightly complains.

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	PR libctf/27628
	* ctf-lookup.c (isqualifier): Don't dereference out-of-bounds
	qhash values.
---
 libctf/ctf-lookup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.31.0.253.gdec51257f3

Comments

Hans-Peter Nilsson March 25, 2021, 12:02 a.m. | #1
On Wed, 24 Mar 2021, Nick Alcock via Binutils wrote:

> diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c

> index 9d1e6d8a4a2..e50c868c5b8 100644

> --- a/libctf/ctf-lookup.c

> +++ b/libctf/ctf-lookup.c

> @@ -111,10 +111,13 @@ isqualifier (const char *s, size_t len)

>    };

>

>    int h = s[len - 1] + (int) len - 105;

> +

> +  if (h < 0 || (size_t) h >= sizeof (qhash) / sizeof (qhash[0]))

> +    return 0;

> +

>    const struct qual *qp = &qhash[h];


Do we allow C99 these days?  In recent messages I got the
impression that we're still battling with pre-C90 artefacts.

If not, watch out for the declaration-after-statement there.

brgds, H-P
Alan Modra via Binutils March 25, 2021, 3:53 p.m. | #2
On 25 Mar 2021, Hans-Peter Nilsson uttered the following:

> On Wed, 24 Mar 2021, Nick Alcock via Binutils wrote:

>

>> diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c

>> index 9d1e6d8a4a2..e50c868c5b8 100644

>> --- a/libctf/ctf-lookup.c

>> +++ b/libctf/ctf-lookup.c

>> @@ -111,10 +111,13 @@ isqualifier (const char *s, size_t len)

>>    };

>>

>>    int h = s[len - 1] + (int) len - 105;

>> +

>> +  if (h < 0 || (size_t) h >= sizeof (qhash) / sizeof (qhash[0]))

>> +    return 0;

>> +

>>    const struct qual *qp = &qhash[h];

>

> Do we allow C99 these days?  In recent messages I got the

> impression that we're still battling with pre-C90 artefacts.

> 

> If not, watch out for the declaration-after-statement there.


We have declaration-after-statements all over libctf, so if people
really do try to compile with a pre-C99 compiler, we'll know (and I'll
fix them all then and growl loudly).

For that matter there are also some in bfd, so it's not just me.

(But this one is totally gratuitous and doesn't even improve clarity, so
I'll fix it :) )

-- 
NULL && (void)

Patch

diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
index 9d1e6d8a4a2..e50c868c5b8 100644
--- a/libctf/ctf-lookup.c
+++ b/libctf/ctf-lookup.c
@@ -111,10 +111,13 @@  isqualifier (const char *s, size_t len)
   };
 
   int h = s[len - 1] + (int) len - 105;
+
+  if (h < 0 || (size_t) h >= sizeof (qhash) / sizeof (qhash[0]))
+    return 0;
+
   const struct qual *qp = &qhash[h];
 
-  return (h >= 0 && (size_t) h < sizeof (qhash) / sizeof (qhash[0])
-	  && (size_t) len == qp->q_len &&
+  return ((size_t) len == qp->q_len &&
 	  strncmp (qp->q_name, s, qp->q_len) == 0);
 }