openssh unix dev - Aug 2018 - Coverity warning about code in smult_curve25519_ref.c

Hi can some one point me to someone who understands how the following
code in smult_curve25519_ref.c

square() is *NOT* a buffer overrun.

I understand that it is called from places with 64 entry arrays but still

 unsigned int c1[64];
....
square(r,c1 + 32); - where c1 is a 64 entry array sounds dangerous.

Where:

static void square(unsigned int out[32],const unsigned int a[32])
{
  unsigned int i;
  unsigned int j;
  unsigned int u;

  for (i = 0;i < 32;++i) {
    u = 0;
    for (j = 0;j < i - j;++j) u += a[j] * a[i - j];
    for (j = i + 1;j < i + 32 - j;++j) u += 38 * a[j] * a[i + 32 - j];
    u *= 2;
    if ((i & 1) == 0) {
      u += a[i / 2] * a[i / 2];
      u += 38 * a[i / 2 + 16] * a[i / 2 + 16];
    }
    out[i] = u;
  }
  squeeze(out);
}

Even if this code is valid - would it not be wise to ask someone to
revise it to be more transparent and easily verifiable?

Thanks

Andrew

I think coverity is wrong and the code in smult_curve25519_ref.c
square()  is ok. After discussing this with some one else I
realised that j is bound by j < i + 32 - j; where starts j = i + 1
hence the worse case is j < 31 (i=31) and the limit gets tigher on
each iteration of the loop.

Sorry to have bothered people. Surprised that Coverity had this wrong.

Andrew
In p
On Mon, 20 Aug 2018 at 12:12, Andrew Worsley <amworsley at gmail.com>
wrote:
>
> Hi can some one point me to someone who understands how the following
> code in smult_curve25519_ref.c
>
> square() is *NOT* a buffer overrun.
>
> I understand that it is called from places with 64 entry arrays but still
>
>  unsigned int c1[64];
> ....
> square(r,c1 + 32); - where c1 is a 64 entry array sounds dangerous.
>
> Where:
>
> static void square(unsigned int out[32],const unsigned int a[32])
> {
>   unsigned int i;
>   unsigned int j;
>   unsigned int u;
>
>   for (i = 0;i < 32;++i) {
>     u = 0;
>     for (j = 0;j < i - j;++j) u += a[j] * a[i - j];
>     for (j = i + 1;j < i + 32 - j;++j) u += 38 * a[j] * a[i + 32 - j];
>     u *= 2;
>     if ((i & 1) == 0) {
>       u += a[i / 2] * a[i / 2];
>       u += 38 * a[i / 2 + 16] * a[i / 2 + 16];
>     }
>     out[i] = u;
>   }
>   squeeze(out);
> }
>
> Even if this code is valid - would it not be wise to ask someone to
> revise it to be more transparent and easily verifiable?
>
> Thanks
>
> Andrew