[ntp:security] findpeer()

Harlan Stenn stenn at nwtime.org
Wed Jan 20 04:41:19 UTC 2016



On 1/19/16 6:47 PM, Danny Mayer wrote:
> On 1/19/2016 6:54 PM, Harlan Stenn wrote:
>> ntp_peer.c's findpeer() function takes 3 arguments:
>>
>> struct peer *
>> findpeer(
>>         struct recvbuf *rbufp,
>>         int             pkt_mode,
>>         int *           action
>>         )
>> {
>>
>> pkt_mode seems to be the cleaned-up mode from rbufp->recv_pkt, in
>> ntp_proto.c
>>
>> Apparently we do inadequate cleaning of the previous good packet in the
>> peer_hash because line 301 of ntp_peer.c (in findpeer()) is:
>>
>>        *action = MATCH_ASSOC(p->hmode, pkt_mode);
>>
>> and that indexes into a 7x7 array.  p->hmode can be a much bigger
>> number, apparently.
> 
> Well that piece of data comes from:
> 
> hismode = (int)PKT_MODE(pkt->li_vn_mode);
> 
> and consists only of 3 bits according to the RFC and I see nothing in
> ntp_proto.c that would change this value. Did I miss something?

pkt_mode comes from hismode.  p->hmode is a u_char.

The more interesting question is "how can a packet that is busted like
this get into the peer hash list?

I'm still wondering if this is an ASSERT-worthy case.

Findpeer doesn't have a way to say "No because the packet is bogus".
We're past that point here - the bogus packet should not have been added
to the hash chain.

-- 
Harlan Stenn <stenn at nwtime.org>
http://networktimefoundation.org - be a member!



More information about the security mailing list