Re: Bug: reading out of bounds in cint_array.c

[Andrew J. Schorr]

Hi,

On Mon, Jan 13, 2020 at 04:32:29PM +0300, Michael Builov wrote:
> I have just spotted a possible read beyond array bounds in
> cint_array_init().
> What if NHAT is defined as 30 in the environment?
> 
> The code:
> 
> static NODE **
> cint_array_init(NODE *symbol, NODE *subs)
> {
> ........
>         /* check relevant environment variables */
>         if ((newval = getenv_long("NHAT")) > 1 && newval < INT32_BIT)
>             NHAT = (unsigned) newval;
>         /* don't allow overflow off the end of the table */
>         if (NHAT >= nelems)
>             NHAT = nelems - 2;
>         THRESHOLD = power_two_table[NHAT + 1];
> ..........
> }
> 
> INT32_BIT == 32
> nelems == 31
> so
> THRESHOLD = power_two_table[30 + 1];   // reading out of bounds!

Thanks for the bug report. I think your analysis is correct.
This patch should fix it:

diff --git a/cint_array.c b/cint_array.c
index 417f27d..d7171ac 100644
--- a/cint_array.c
+++ b/cint_array.c
@@ -171,15 +171,15 @@ cint_array_init(NODE *symbol ATTRIBUTE_UNUSED, NODE *subs 
ATTRIBUTE_UNUSED)
                long newval;
                size_t nelems = (sizeof(power_two_table) / 
sizeof(power_two_table[0]));
 
                /* check relevant environment variables */
                if ((newval = getenv_long("NHAT")) > 1 && newval < INT32_BIT)
                        NHAT = newval;
                /* don't allow overflow off the end of the table */
-               if (NHAT >= nelems)
+               if (NHAT > nelems - 2)
                        NHAT = nelems - 2;
                THRESHOLD = power_two_table[NHAT + 1];
        } else
                null_array(symbol);
 
        return & success_node;
 }


Unless there's a deeper logic flaw elsewhere related to NHAT usage. I haven't
reviewed whether there was some twisted and mistaken logic behind the original
error that might point to problems elsewhere...

Regards,
Andy