[ntp:questions] ntpdate.c unsafe buffer write

Unruh unruh-spam at physics.ubc.ca
Fri Feb 8 22:20:48 UTC 2008


"David L. Mills" <mills at udel.edu> writes:

>Harlan,

>My position on ntpdate and sntp has always been clear. Remove them both 
>from the distribution and let other folks contribute sntp products. The 
>standards labs in various contries do not recommend the NTP reference 
>implementation, they recommend other shrinkwrap products. There is no 
>need for folks to download the reference implementatino only to bring up 
>an sntp product.

Surely supplying a properly working sntp client is better than relying on
others to do so ( or rather not to do so). Ie, you write a client which
works well, and since you are the ntp source, yours wins out over the bad
ones. Otherwise you rely on the others to badly impliment the protocol.


>The matter of concern is an sntp product that strictly conforms to the 
>NTPv4 specification as it applies to sntp. There is at least one 
>contributor testing the kiss-o'-death rate limit and has apparently 
>actually read rfc 2030. On the other hand, there are numerous examples 
>of clients that casually violate the rate rules both at servers we 
>operate here and at the national labs. What we should be doing is 
>supporting those products that play by the rules and that are maintained 
>by other players.

>Dave

>Harlan Stenn wrote:
>> Bill,
>> 
>> ntpdate is being deprecated.
>> 
>> And it is *much* better to file reports like this using bugs.ntp.org as
>> otherwise they tend to get lost in the wind.
>> 
>> H
>> --
>> 
>>>>>In article <4FIqj.1315$FO1.16 at edtnps82>, Unruh <unruh-spam at physics.ubc.ca> writes:
>> 
>> 
>> Unruh> In ntpdate.c around line 542 (4.2.4p4)is the sequence if
>> Unruh> (!authistrusted(sys_authkey)) { char buf[10];
>> 
>> Unruh>          (void) sprintf(buf, "%lu", (unsigned long)sys_authkey);
>> Unruh> msyslog(LOG_ERR, "authentication key %s unknown", buf); exit(1);
>> Unruh> }
>> 
>> Unruh> Since unsigned long does not have a definite length on all machines,
>> Unruh> and with the trailing zero certainly is potentially longer than 10
>> Unruh> bytes, that buf is ripe for buffer overflow.  It should be something
>> Unruh> like char buf[(sizeof(unsigned long)*12/5+2)]; And/or the sprintf
>> Unruh> should be an snprintf.
>> 
>> 
>> 




More information about the questions mailing list