From: Harlan Stenn <stenn@ntp.org>
Date: Sat, 15 Apr 2023 11:41:21 +0000 (-0500)
Subject: Bug 3807: praecis_parse() input buffer
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a37d77de89a6c052b008ca60ff92469e1b71dfaf;p=thirdparty%2Fntp.git

Bug 3807: praecis_parse() input buffer

bk: 643a8d61y9UILY8OQMvIryixOrH57g
---

diff --git a/ChangeLog b/ChangeLog
index d6af39782..9e53d9b35 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,6 @@
 ---
+* [Sec 3807] praecis_parse() in the Palisade refclock driver has a
+             hypothetical input buffer overflow. Reported by ... stenn@
 * [Bug 3802] ntp-keygen -I default identity modulus bits too small for
              OpenSSL 3.  Reported by rmsh1216@163.com <hart@ntp.org>
 * [Bug 3801] gpsdjson refclock gps_open() device name mishandled. <hart@ntp.org>
diff --git a/ntpd/refclock_palisade.c b/ntpd/refclock_palisade.c
index 7c8290fb2..bfe11e2aa 100644
--- a/ntpd/refclock_palisade.c
+++ b/ntpd/refclock_palisade.c
@@ -1249,20 +1249,53 @@ praecis_parse (
 
 	pp = peer->procptr;
 
-	memcpy(buf+p,rbufp->recv_space.X_recv_buffer, rbufp->recv_length);
+	if (p + rbufp->recv_length >= sizeof buf) {
+		struct palisade_unit *up;
+		up = pp->unitptr;
+
+		/*
+		 * We COULD see if there is a \r\n in the incoming
+		 * buffer before it overflows, and then process the
+		 * current line.
+		 *
+		 * Similarly, if we already have a hunk of data that
+		 * we're now flushing, that will cause the line of
+		 * data we're in the process of collecting to be garbage.
+		 *
+		 * Since we now check for this overflow and log when it
+		 * happens, we're now in a better place to easily see
+		 * what's going on and perhaps better choices can be made.
+		 */
+
+		/* Do we need to log the size of the overflow? */
+		msyslog(LOG_ERR, "Palisade(%d) praecis_parse(): input buffer overflow", 
+			up->unit);
+
+		p = 0;
+		praecis_msg = 0;
+
+		refclock_report(peer, CEVNT_BADREPLY);
+
+		return;
+	}
+
+	memcpy(buf+p, rbufp->recv_space.X_recv_buffer, rbufp->recv_length);
 	p += rbufp->recv_length;
 
-	if(buf[p-2] == '\r' && buf[p-1] == '\n') {
+	if (   p >= 2
+	    && buf[p-2] == '\r' 
+	    && buf[p-1] == '\n') {
 		buf[p-2] = '\0';
 		record_clock_stats(&peer->srcadr, buf);
 
 		p = 0;
 		praecis_msg = 0;
 
-		if (HW_poll(pp) < 0)
+		if (HW_poll(pp) < 0) {
 			refclock_report(peer, CEVNT_FAULT);
-
+		}
 	}
+	return;
 }
 
 static void