Bug 3807: praecis_parse() input buffer

bk: 643a8d61y9UILY8OQMvIryixOrH57g

diff --git a/ChangeLog b/ChangeLog
index d6af397823ae357316b275bbb0cb9ad83621b204..9e53d9b354e03a1ca9ab33e5aafe6f253d408b63 100644 (file)
--- 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 7c8290fb2f18216ad076b77ae2befc5efee6c5a0..bfe11e2aac76c6b1653ab19d6621dc6fae6cb589 100644 (file)
--- 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