On Saturday 22 March 2014 23:46:44 Isaac Dunham wrote:
> On Sat, Mar 22, 2014 at 08:40:48PM +0100, Harald Becker wrote:
> > Hi Isaac !
> > 
> > Your program will fail on lines starting with the word server
> > (eg. serverxyz), that is it does not check for clear word
> > boundary and gives wrong results in that case.
> 
> ...which are not legitimate entries in ntp.conf.
> 
> My aim is to parse a correct ntp.conf, and not cause security problems
> on incorrect ones.
> 
> > >while (cbuf[i] > 35) i++;
> > 
> > Unwise to do this in a not poor ASCII environment, as most
> > systems are nowadays. This way you allow unprintable and any
> > kind of illegal characters in time server addresses. 
> Fixing. 
> The fix expects chars exclusively in the set [-.:0-9a-zA-Z], which all 
> valid hostnames and IP addresses (ipv4/ipv6) have.
> 
> >... and what
> > about buffer overflow? Won't this loop then run to unknown
> > locations?
> 
> Not possible. i is size_t, and getline() is _always_ \0 terminated.
> However, the previous loop did have a potential buffer overrun if the
> line ended prematurely:
> server \n\0 would result in it walking over the end and writing 0 to the
> first character less than 36 after a sequence of chars greater than
> 35 ('#')...
> 
> > Beside this: Make it a default NO configuration, not being
> > included in binaries build from standard options.
> 
> OK. 
> (Denys gets the final say on that, though.)
> 
> Here's a version that has the issues mentioned fixed, and removes the 10
> byte overhead.
> It accepts "peer" as well as "server", and runs 320 bytes.
> 
> 
> Thanks,
> Isaac Dunham
> 
HI,
couldn't this parser use bb's parse infrastructure in libbb/parse_config.c?

///config:        Typical usage of parse API:
////config:             char *t[3];
////config:             parser_t *p = config_open(filename);
////config:             while (config_read(p, t, 3, 0, delimiters, flags)) { // 
1..3 tokens
////config:                     bb_error_msg("TOKENS: '%s''%s''%s'", t[0], 
t[1], t[2]);
////config:             }
////config:             config_close(p);


I think /etc/ntp.conf string should be moved to libbb.h.

Ciao,
Tito 
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to