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
>From 6c825a8e6c83715306f82540e540bac8d48af64f Mon Sep 17 00:00:00 2001
From: Isaac Dunham <[email protected]>
Date: Sat, 22 Mar 2014 11:44:30 -0700
Subject: [PATCH] ntpd: parse /etc/ntp.conf
This is a rudimentary parser for /etc/ntp.conf; it only looks for
lines like this:
server some.pool.ntp.org
peer 0.0.0.0
All options are ignored.
+0 bytes when disabled.
Enabled, it's +320 bytes with gcc 4.4 and glibc 2.11.3:
function old new delta
ntp_init 406 593 +187
add_peers - 107 +107
.rodata 141382 141402 +20
packed_usage 29386 29392 +6
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 3/0 up/down: 320/0) Total: 320 bytes
text data bss dec hex filename
759413 2059 9020 770492 bc1bc busybox_old
759727 2059 9020 770806 bc2f6 busybox_unstripped
---
networking/Config.src | 8 ++++++++
networking/ntpd.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/networking/Config.src b/networking/Config.src
index ca0ddcd..ec50134 100644
--- a/networking/Config.src
+++ b/networking/Config.src
@@ -664,6 +664,14 @@ config FEATURE_NTPD_SERVER
Make ntpd usable as a NTP server. If you disable this option
ntpd will be usable only as a NTP client.
+config FEATURE_NTPD_CONF
+ bool "Make ntpd understand /etc/ntp.conf"
+ default n
+ depends on NTPD
+ help
+ Make ntpd look in /etc/ntp.conf for peers. Only "server address"
+ and "peer address" are supported.
+
config PSCAN
bool "pscan"
default y
diff --git a/networking/ntpd.c b/networking/ntpd.c
index 44592ce..b65bb91 100644
--- a/networking/ntpd.c
+++ b/networking/ntpd.c
@@ -42,6 +42,9 @@
//usage: )
//usage: "\n -S PROG Run PROG after stepping time, stratum change, and every 11 mins"
//usage: "\n -p PEER Obtain time from PEER (may be repeated)"
+//usage: IF_FEATURE_NTPD_CONF(
+//usage: "\nIf -p is not specified, look in /etc/ntp.conf"
+//usage: )
#include "libbb.h"
#include <math.h>
@@ -2087,14 +2090,44 @@ static NOINLINE void ntp_init(char **argv)
"d" /* compat */
"46aAbgL", /* compat, ignored */
&peers, &G.script_name, &G.verbose);
+#if ENABLE_FEATURE_NTPD_CONF
+#else
if (!(opts & (OPT_p|OPT_l)))
bb_show_usage();
+#endif
// if (opts & OPT_x) /* disable stepping, only slew is allowed */
// G.time_was_stepped = 1;
if (peers) {
while (peers)
add_peers(llist_pop(&peers));
} else {
+#if ENABLE_FEATURE_NTPD_CONF
+ size_t bsiz = 0, i=0;
+ char *buf = 0, *cbuf;
+ FILE *stream = fopen("/etc/ntp.conf","r");
+
+ while (stream && !errno) {
+ getline(&buf, &bsiz, stream);
+ cbuf = buf;
+ if (!strncmp(buf, "server", 6)) cbuf += 6;
+ if (!strncmp(buf, "peer", 4)) cbuf += 4;
+ if (cbuf > buf){
+ while (cbuf[0] <= ' ' && cbuf[0]) cbuf++;
+ while (cbuf[i]=='.' || cbuf[i]=='-' ||
+ cbuf[i]==':' || isalnum(cbuf[i]))
+ i++;
+ cbuf[i] = 0;
+ add_peers(xstrdup(cbuf));
+ }
+ }
+ if (buf) free(buf);
+ if (stream) fclose(stream);
+ errno = 0;
+ }
+ if (!(opts & OPT_l) && !G.peer_cnt) {
+ bb_show_usage();
+ } else if (!G.peer_cnt) {
+#endif
/* -l but no peers: "stratum 1 server" mode */
G.stratum = 1;
}
--
1.7.10.4
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
