[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Bug: reading out of bounds in cint_array.c
From: |
arnold |
Subject: |
Re: Bug: reading out of bounds in cint_array.c |
Date: |
Tue, 14 Jan 2020 02:55:39 -0700 |
User-agent: |
Heirloom mailx 12.5 7/5/10 |
Michael,
Thanks for the report.
Andy - please push that to master (after checking make check and
make valgrind).
Thanks to both of you!
Arnold
"Andrew J. Schorr" <address@hidden> wrote:
> 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