[ntp:security] [Bug 2671] vallen is not validated, leading to potential info leak

bugzilla-daemon at ntp.org bugzilla-daemon at ntp.org
Sat Jan 3 13:04:26 UTC 2015


--- Comment #4 from Stephen Röttger <stephen.roettger at gmail.com> 2015-01-03 13:04:26 UTC ---
> The reported problem at line 571 is now at line 601, and I believe the check at
> line 529 handles this case.  Do you agree?

yes, looks good to me.

> The CERT|RESP case that was at 1162 is now near line 1209, and I believe I have
> fixed that around line 1198.

looks good to me.

> The reported problem in crypto_verify() that was at 1461 in the old code is at
> (I believe) line 1561 in the current code.  I believe the check at line 1502
> handles this case already.  Do you agree?

The code looks like this:
    i = (vallen + 3) / 4;
    siglen = ntohl(ep->pkt[i++]);
    if (len < VALUE_LEN + ((vallen + 3) / 4) * 4 + ((siglen + 3) /
        4) * 4)
        return (XEVNT_LEN);
"i" is calculated from vallen and used as a read index to ep->pkt. Are there
any checks that I'm missing? Also, the calculations in the check afterwards can
overflow, for example if vallen is UINT32_MAX-3, the result will be 0 and pass
the check.

> The reported problem in crypto_encrypt() that was at line 1559 in the original
> code is now at line 1599 in the new code.  I think I have this fixed with the
new test at line 1350.  Do you agree?

looks good to me

> The reported problem that was at line 2117 of crypto_bob() in the old code is
> now around line 2187 of the new code.  I think I have fixed this with the new
> test at line 2189.  Do you agree?

looks good to me

What about crypto_bob2? That one looks similar to crypto_bob. I think it would
be the better approach to verify the packet format once in the beginning
instead of doing it every time a value is used. This way it's easy to miss
something and it makes it very tough to verify. Is there a reason you decided
against that approach?

Configure bugmail: http://bugs.ntp.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

More information about the security mailing list