2017-02-12 08:03:30+01:00, perlinger@ntp.org
  [Sec 3379] NTP-01-004 Potential Overflows in ctl_put() functions

diff -up ntp-4.2.6p5/ntpd/ntp_control.c.x1 ntp-4.2.6p5/ntpd/ntp_control.c
--- ntp-4.2.6p5/ntpd/ntp_control.c.x1	2017-03-22 15:53:49.658708555 +0100
+++ ntp-4.2.6p5/ntpd/ntp_control.c	2017-03-22 17:40:09.705771172 +0100
@@ -50,8 +50,7 @@ static	u_short ctlclkstatus	(struct refc
 #endif
 static	void	ctl_flushpkt	(int);
 static	void	ctl_putdata	(const char *, unsigned int, int);
-static	void	ctl_putstr	(const char *, const char *,
-				 unsigned int);
+static	void	ctl_putstr	(const char *, const char *, size_t);
 static	void	ctl_putdbl	(const char *, double);
 static	void	ctl_putuint	(const char *, u_long);
 static	void	ctl_puthex	(const char *, u_long);
@@ -1052,27 +1051,19 @@ static void
 ctl_putstr(
 	const char *tag,
 	const char *data,
-	unsigned int len
+	size_t len
 	)
 {
-	register char *cp;
-	register const char *cq;
-	char buffer[400];
+	char buffer[512];
+	int  rc;
 
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-	if (len > 0) {
-		*cp++ = '=';
-		*cp++ = '"';
-		if (len > (int) (sizeof(buffer) - (cp - buffer) - 1))
-			len = sizeof(buffer) - (cp - buffer) - 1;
-		memmove(cp, data, (unsigned)len);
-		cp += len;
-		*cp++ = '"';
-	}
-	ctl_putdata(buffer, (unsigned)( cp - buffer ), 0);
+
+	if (len)
+	    rc = snprintf(buffer, sizeof(buffer), "%s=\"%.*s\"", tag, (int)len, data);
+	else
+	    rc = snprintf(buffer, sizeof(buffer), "%s", tag);
+
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1082,22 +1073,15 @@ ctl_putstr(
 static void
 ctl_putdbl(
 	const char *tag,
-	double ts
+ 	double d
 	)
 {
-	register char *cp;
-	register const char *cq;
 	char buffer[200];
-
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-	*cp++ = '=';
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer), "%.3f", ts);
-	cp += strlen(cp);
-	ctl_putdata(buffer, (unsigned)(cp - buffer), 0);
+	int  rc;
+	
+	rc = snprintf(buffer, sizeof(buffer), "%s=%.3f", tag, d);
+	INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 /*
@@ -1109,20 +1093,12 @@ ctl_putuint(
 	u_long uval
 	)
 {
-	register char *cp;
-	register const char *cq;
 	char buffer[200];
+	int  rc;
 
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-
-	*cp++ = '=';
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer), "%lu", uval);
-	cp += strlen(cp);
-	ctl_putdata(buffer, (unsigned)( cp - buffer ), 0);
+	rc = snprintf(buffer, sizeof(buffer), "%s=%lu", tag, uval);
+	INSIST(rc >= 0 && rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 /*
@@ -1134,28 +1110,23 @@ ctl_putfs(
 	tstamp_t uval
 	)
 {
-	register char *cp;
-	register const char *cq;
 	char buffer[200];
 	struct tm *tm = NULL;
 	time_t fstamp;
-
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-
-	*cp++ = '=';
-	fstamp = uval - JAN_1970;
+	int    rc;
+	
+	fstamp = (time_t)uval - JAN_1970;
 	tm = gmtime(&fstamp);
-	if (NULL ==  tm)
+	if (NULL == tm)
 		return;
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer),
-		 "%04d%02d%02d%02d%02d", tm->tm_year + 1900,
-		 tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min);
-	cp += strlen(cp);
-	ctl_putdata(buffer, (unsigned)( cp - buffer ), 0);
+
+	rc = snprintf(buffer, sizeof(buffer),
+		      "%s=%04d%02d%02d%02d%02d",
+		      tag,
+		      tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
+		      tm->tm_hour, tm->tm_min);
+	INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1169,20 +1140,12 @@ ctl_puthex(
 	u_long uval
 	)
 {
-	register char *cp;
-	register const char *cq;
 	char buffer[200];
-
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-
-	*cp++ = '=';
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer), "0x%lx", uval);
-	cp += strlen(cp);
-	ctl_putdata(buffer,(unsigned)( cp - buffer ), 0);
+	int  rc;
+	
+	rc = snprintf(buffer, sizeof(buffer), "%s=0x%lx", tag, uval);
+	INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1195,20 +1158,12 @@ ctl_putint(
 	long ival
 	)
 {
-	register char *cp;
-	register const char *cq;
 	char buffer[200];
-
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-
-	*cp++ = '=';
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer), "%ld", ival);
-	cp += strlen(cp);
-	ctl_putdata(buffer, (unsigned)( cp - buffer ), 0);
+	int  rc;
+	
+	rc = snprintf(buffer, sizeof(buffer), "%s=%ld", tag, ival);
+	INSIST(rc >= 0 && rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1221,21 +1176,14 @@ ctl_putts(
 	l_fp *ts
 	)
 {
-	register char *cp;
-	register const char *cq;
 	char buffer[200];
-
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-
-	*cp++ = '=';
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer), "0x%08lx.%08lx",
-		 ts->l_ui & 0xffffffffUL, ts->l_uf & 0xffffffffUL);
-	cp += strlen(cp);
-	ctl_putdata(buffer, (unsigned)( cp - buffer ), 0);
+	int  rc;
+	
+	rc = snprintf(buffer, sizeof(buffer),
+		      "%s=0x%08lx.%08lx",
+		      tag, (u_long)ts->l_ui, (u_long)ts->l_uf);
+	INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1249,24 +1197,17 @@ ctl_putadr(
 	sockaddr_u *addr
 	)
 {
-	register char *cp;
-	register const char *cq;
+	const char *cq;
 	char buffer[200];
-
-	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
-
-	*cp++ = '=';
+	int  rc;
+	
 	if (NULL == addr)
 		cq = numtoa(addr32);
 	else
 		cq = stoa(addr);
-	NTP_INSIST((cp - buffer) < sizeof(buffer));
-	snprintf(cp, sizeof(buffer) - (cp - buffer), "%s", cq);
-	cp += strlen(cp);
-	ctl_putdata(buffer, (unsigned)(cp - buffer), 0);
+	rc = snprintf(buffer, sizeof(buffer), "%s=%s", tag, cq);
+	INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1279,34 +1220,22 @@ ctl_putrefid(
 	u_int32		refid
 	)
 {
-	char	output[16];
-	char *	optr;
-	char *	oplim;
-	char *	iptr;
-	char *	iplim;
-	char *	past_eq;
-
-	optr = output;
-	oplim = output + sizeof(output);
-	while (optr < oplim && '\0' != *tag)
-		*optr++ = *tag++;
-	if (optr < oplim) {
-		*optr++ = '=';
-		past_eq = optr;
-	}
-	if (!(optr < oplim))
-		return;
-	iptr = (char *)&refid;
-	iplim = iptr + sizeof(refid);
-	for ( ; optr < oplim && iptr < iplim && '\0' != *iptr; 
-	     iptr++, optr++)
-		if (isprint(*iptr))
-			*optr = *iptr;
-		else
-			*optr = '.';
-	if (!(optr <= oplim))
-		optr = past_eq;
-	ctl_putdata(output, (u_int)(optr - output), FALSE);
+	char buffer[128];
+	int  rc, i;
+
+	union {
+		uint32_t w;
+		uint8_t  b[sizeof(uint32_t)];
+	} bytes;
+
+	bytes.w = refid;
+	for (i = 0; i < sizeof(bytes.b); ++i)
+		if (bytes.b[i] && !isprint(bytes.b[i]))
+			bytes.b[i] = '.';
+	rc = snprintf(buffer, sizeof(buffer), "%s=%.*s",
+		      tag, (int)sizeof(bytes.b), bytes.b);
+	INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+	ctl_putdata(buffer, (u_int)rc, FALSE);
 }
 
 
@@ -1320,25 +1249,27 @@ ctl_putarray(
 	int start
 	)
 {
-	register char *cp;
-	register const char *cq;
+	char *cp, *ep;
 	char buffer[200];
-	int i;
+	int  i, rc;
+
 	cp = buffer;
-	cq = tag;
-	while (*cq != '\0')
-		*cp++ = *cq++;
+	ep = buffer + sizeof(buffer);
+
+	rc  = snprintf(cp, (size_t)(ep - cp), "%s=", tag);
+	INSIST(rc >= 0 && rc < (ep - cp));
+	cp += rc;
+
 	i = start;
 	do {
 		if (i == 0)
 			i = NTP_SHIFT;
 		i--;
-		NTP_INSIST((cp - buffer) < sizeof(buffer));
-		snprintf(cp, sizeof(buffer) - (cp - buffer),
-			 " %.2f", arr[i] * 1e3);
-		cp += strlen(cp);
-	} while(i != start);
-	ctl_putdata(buffer, (unsigned)(cp - buffer), 0);
+		rc = snprintf(cp, (size_t)(ep - cp), " %.2f", arr[i] * 1e3);
+		INSIST(rc >= 0 && rc < (ep - cp));
+		cp += rc;
+	} while (i != start);
+	ctl_putdata(buffer, (u_int)(cp - buffer), 0);
 }