[Sec 3378] NTP-01-003 Improper use of snprintf() in mx4200_send()

bk: 589f6a59geVwfxo2jMu6V8GxzwUENQ

diff --git a/ChangeLog b/ChangeLog
index 595a3d77629ef0c056a6c4fb26f723863cf7d42e..a34b35edd3788ee2fbc5e6cc9c20733c35559e5e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+---
+* [Sec 3378] NTP-01-003 Improper use of snprintf() in mx4200_send()
+  (Pentest report 01.2017) <perlinger@ntp.org>
+
 ---
 (4.2.8p9-win) 2017/02/01 Released by Harlan Stenn <stenn@ntp.org>
 

diff --git a/ntpd/refclock_mx4200.c b/ntpd/refclock_mx4200.c
index c9422290d39a449fb5333ec8c1fa22376643207d..6969e6a2deb1814f8cc891e6a5ffbdb55d53e5d4 100644 (file)
--- a/ntpd/refclock_mx4200.c
+++ b/ntpd/refclock_mx4200.c
@@ -1596,34 +1596,42 @@ mx4200_send(peer, fmt, va_alist)
        struct refclockproc *pp;
        struct mx4200unit *up;
 
-       register char *cp;
+       register char *cp, *ep;
        register int n, m;
        va_list ap;
        char buf[1024];
        u_char ck;
 
+       pp = peer->procptr;
+       up = pp->unitptr;
+
+       cp = buf;
+       ep = cp + sizeof(buf);
+       *cp++ = '$';
+       
 #if defined(__STDC__)
        va_start(ap, fmt);
 #else
        va_start(ap);
 #endif /* __STDC__ */
+       n = VSNPRINTF((cp, (size_t)(ep - cp), fmt, ap));
+       va_end(ap);
+       if (n < 0 || (size_t)n >= (size_t)(ep - cp))
+               goto overflow;
 
-       pp = peer->procptr;
-       up = pp->unitptr;
-
-       cp = buf;
-       *cp++ = '$';
-       n = VSNPRINTF((cp, sizeof(buf) - 1, fmt, ap));
        ck = mx4200_cksum(cp, n);
+       cp += n;            
+       n = SNPRINTF((cp, (size_t)(ep - cp), "*%02X\r\n", ck));
+       if (n < 0 || (size_t)n >= (size_t)(ep - cp))
+               goto overflow;
        cp += n;
-       ++n;
-       n += SNPRINTF((cp, sizeof(buf) - n - 5, "*%02X\r\n", ck));
-
-       m = write(pp->io.fd, buf, (unsigned)n);
+       m = write(pp->io.fd, buf, (unsigned)(cp - buf));
        if (m < 0)
                msyslog(LOG_ERR, "mx4200_send: write: %m (%s)", buf);
        mx4200_debug(peer, "mx4200_send: %d %s\n", m, buf);
-       va_end(ap);
+       
+  overflow:
+       msyslog(LOG_ERR, "mx4200_send: %s", "data exceeds buffer size");
 }
 
 #else