From: Juergen Perlinger <perlinger@ntp.org>
Date: Thu, 8 Dec 2016 07:28:15 +0000 (+0100)
Subject: [Bug 2645] out-of-bound pointers in ctl_putsys and decode_bitflags
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2c9e8414411ff09441263c61e5f60c30d22d79ab;p=thirdparty%2Fntp.git

[Bug 2645] out-of-bound pointers in ctl_putsys and decode_bitflags

bk: 58490b8fuFDR6QJNxvoteVgJshkjYQ
---

diff --git a/ChangeLog b/ChangeLog
index 0805467dc..fea1f55bc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+---
+* [Bug 2645] out-of-bound pointers in ctl_putsys and decode_bitflags
+  - Fixed these and some more locations of this pattern.
+    Probably din't get them all, though. <perlinger@ntp.org>
+
 ---
 (4.2.8p8) 2016/06/02 Released by Harlan Stenn <stenn@ntp.org>
 
diff --git a/lib/isc/inet_pton.c b/lib/isc/inet_pton.c
index eac631b58..7e0766726 100644
--- a/lib/isc/inet_pton.c
+++ b/lib/isc/inet_pton.c
@@ -170,7 +170,7 @@ inet_pton6(const char *src, unsigned char *dst) {
 				colonp = tp;
 				continue;
 			}
-			if (tp + NS_INT16SZ > endp)
+			if (NS_INT16SZ > endp - tp)
 				return (0);
 			*tp++ = (unsigned char) (val >> 8) & 0xff;
 			*tp++ = (unsigned char) val & 0xff;
@@ -178,7 +178,7 @@ inet_pton6(const char *src, unsigned char *dst) {
 			val = 0;
 			continue;
 		}
-		if (ch == '.' && ((tp + NS_INADDRSZ) <= endp) &&
+		if (ch == '.' && (NS_INADDRSZ <= endp - tp) &&
 		    inet_pton4(curtok, tp) > 0) {
 			tp += NS_INADDRSZ;
 			seen_xdigits = 0;
@@ -187,7 +187,7 @@ inet_pton6(const char *src, unsigned char *dst) {
 		return (0);
 	}
 	if (seen_xdigits) {
-		if (tp + NS_INT16SZ > endp)
+		if (NS_INT16SZ > endp - tp)
 			return (0);
 		*tp++ = (unsigned char) (val >> 8) & 0xff;
 		*tp++ = (unsigned char) val & 0xff;
diff --git a/libntp/statestr.c b/libntp/statestr.c
index 313cd46be..b8fa53ccb 100644
--- a/libntp/statestr.c
+++ b/libntp/statestr.c
@@ -355,13 +355,12 @@ decode_bitflags(
 
 	for (b = 0; b < tab_ct; b++) {
 		if (tab[b].code & bits) {
-			rc = snprintf(pch, (lim - pch), "%s%s", sep,
+			size_t avail = lim - pch;
+			rc = snprintf(pch, avail, "%s%s", sep,
 				      tab[b].string);
-			if (rc < 0)
-				goto toosmall;
-			pch += (u_int)rc;
-			if (pch >= lim)
+			if ((size_t)rc >= avail)
 				goto toosmall;
+			pch += rc;
 			sep = sep2;
 		}
 	}
diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
index 07b5697f1..8a692b4fc 100644
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -1816,7 +1816,7 @@ ctl_putadr(
 		cq = numtoa(addr32);
 	else
 		cq = stoa(addr);
-	INSIST((cp - buffer) < (int)sizeof(buffer));
+	INSIST((size_t)(cp - buffer) < sizeof(buffer));
 	snprintf(cp, sizeof(buffer) - (cp - buffer), "%s", cq);
 	cp += strlen(cp);
 	ctl_putdata(buffer, (unsigned)(cp - buffer), 0);
@@ -2086,7 +2086,7 @@ ctl_putsys(
 
 		buffp = buf;
 		buffend = buf + sizeof(buf);
-		if (buffp + strlen(sys_var[CS_VARLIST].text) + 4 > buffend)
+		if (strlen(sys_var[CS_VARLIST].text) + 4 > buffend - buffp)
 			break;	/* really long var name */
 
 		snprintf(buffp, sizeof(buf), "%s=\"",sys_var[CS_VARLIST].text);
@@ -2096,7 +2096,7 @@ ctl_putsys(
 			if (k->flags & PADDING)
 				continue;
 			len = strlen(k->text);
-			if (buffp + len + 1 >= buffend)
+			if (len + 1 >= buffend - buffp)
 				break;
 			if (!firstVarName)
 				*buffp++ = ',';
@@ -2116,7 +2116,7 @@ ctl_putsys(
 				len = strlen(k->text);
 			else
 				len = ss1 - k->text;
-			if (buffp + len + 1 >= buffend)
+			if (len + 1 >= buffend - buffp)
 				break;
 			if (firstVarName) {
 				*buffp++ = ',';
@@ -2125,7 +2125,7 @@ ctl_putsys(
 			memcpy(buffp, k->text,(unsigned)len);
 			buffp += len;
 		}
-		if (buffp + 2 >= buffend)
+		if (2 >= buffend - buffp)
 			break;
 
 		*buffp++ = '"';
diff --git a/ntpd/ntp_io.c b/ntpd/ntp_io.c
index 80642524e..759cfe7bf 100644
--- a/ntpd/ntp_io.c
+++ b/ntpd/ntp_io.c
@@ -2561,7 +2561,7 @@ io_setbclient(void)
 {
 #ifdef OPEN_BCAST_SOCKET
 	struct interface *	interf;
-	int			nif;
+	unsigned int		nif;
 
 	nif = 0;
 	set_reuseaddr(1);
@@ -2638,11 +2638,10 @@ io_setbclient(void)
 		}
 	}
 	set_reuseaddr(0);
-	if (nif > 0) {
+	if (nif != 0) {
 		broadcast_client_enabled = ISC_TRUE;
 		DPRINTF(1, ("io_setbclient: listening to %d broadcast addresses\n", nif));
-	}
-	else if (!nif) {
+	} else {
 		broadcast_client_enabled = ISC_FALSE;
 		msyslog(LOG_ERR,
 			"Unable to listen for broadcasts, no broadcast interfaces available");
diff --git a/ntpdc/ntpdc_ops.c b/ntpdc/ntpdc_ops.c
index 2f57768b4..1a400ec9b 100644
--- a/ntpdc/ntpdc_ops.c
+++ b/ntpdc/ntpdc_ops.c
@@ -2108,7 +2108,7 @@ reset(
 		if (sreset[i].flag == 0) {
 			fprintf(fp, "Flag %s unknown\n",
 				pcmd->argval[res].string);
-			err++;
+			err = 1;
 		} else {
 			rflags.flags |= sreset[i].flag;
 		}
diff --git a/sntp/crypto.c b/sntp/crypto.c
index a534239a3..18a99403e 100644
--- a/sntp/crypto.c
+++ b/sntp/crypto.c
@@ -64,8 +64,8 @@ auth_md5(
 		 * with.  sntp is a 1-shot program, so snooping for
 		 * timing attacks is Harder.
 		 */
-		authentic = !memcmp(digest, pkt_data + pkt_size + 4,
-				    hash_len);
+	    authentic = !memcmp(digest, (const char*)pkt_data + pkt_size + 4,
+				hash_len);
 	}
 	return authentic;
 }