Re: [PATCH] Ntpd config file support
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. 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. ... and what about buffer overflow? Won't this loop then run to unknown locations? Beside this: Make it a default NO configuration, not being included in binaries build from standard options. -- Harald ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Ntpd config file support
Harald Becker wrote: 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. The program will not fail for serverxyz, it will add a server xyz. This may be a bug or a feature :-) 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. What is special about 35? Why is ' fine while is not? Of course the configuration comes from someone who already has root access, so whatever happens here to an invalid input can't be worse than rm -rf /. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Ntpd config file support
On Sat, Mar 22, 2014 at 08:59:28PM +0100, Ralf Friedl wrote: Harald Becker wrote: 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. The program will not fail for serverxyz, it will add a server xyz. This may be a bug or a feature :-) 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. What is special about 35? Why is ' fine while is not? Of course the configuration comes from someone who already has root access, so whatever happens here to an invalid input can't be worse than rm -rf /. Not necessarily. The ntp.conf file might be populated by some kind of autoconfig sysem (does DHCP have a way of offering suggested ntp servers?) which could in turn have malicious content injected. Whose responsibility it is to avoid this is an open question, but I think it's harmful to add a sloppy parser to busybox. If there's going to be a config parser it should attempt to be safe and correct. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Ntpd config file support
On 22 March 2014 21:27:51 Rich Felker dal...@aerifal.cx wrote: On Sat, Mar 22, 2014 at 08:59:28PM +0100, Ralf Friedl wrote: Harald Becker wrote: 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. The program will not fail for serverxyz, it will add a server xyz. This may be a bug or a feature :-) 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. What is special about 35? Why is ' fine while is not? Of course the configuration comes from someone who already has root access, so whatever happens here to an invalid input can't be worse than rm -rf /. Not necessarily. The ntp.conf file might be populated by some kind of autoconfig sysem (does DHCP have a way of offering suggested ntp servers?) which could in turn have malicious content injected. Whose responsibility it is to avoid this is an open question, but I think it's harmful to add a sloppy parser to busybox. If there's going to be a config parser it should attempt to be safe and correct. We already have one, see parse_config / config_parse (forgot which one).. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox Sent with AquaMail for Android http://www.aqua-mail.com ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
