[Sec 3379] NTP-01-004 Potential Overflows in ctl_put() functions

bk: 58a008c2PtxGYR8g6fLpoaFecrZ-zQ

diff --git a/ChangeLog b/ChangeLog
index 595a3d77629ef0c056a6c4fb26f723863cf7d42e..d983ec5e0b99e010e853b5323a46b5c229a5bf8f 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+---
+* [Sec 3379] NTP-01-004 Potential Overflows in ctl_put() functions
+  (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/ntp_control.c b/ntpd/ntp_control.c
index 49a3297c6dd0f8d39c71cdadc8a51fe58e6b13d3..9886ce4933e95eb14098ac4a5863b649e0b656a2 100644 (file)
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -1547,21 +1547,15 @@ ctl_putstr(
        )
 {
        char buffer[512];
-       char *cp;
-       size_t tl;
-
-       tl = strlen(tag);
-       memcpy(buffer, tag, tl);
-       cp = buffer + tl;
-       if (len > 0) {
-               INSIST(tl + 3 + len <= sizeof(buffer));
-               *cp++ = '=';
-               *cp++ = '"';
-               memcpy(cp, data, len);
-               cp += len;
-               *cp++ = '"';
-       }
-       ctl_putdata(buffer, (u_int)(cp - buffer), 0);
+       int  rc;
+
+       INSIST(len < sizeof(buffer));
+       if (len)
+           rc = snprintf(buffer, sizeof(buffer), "%s=\"%.*s\"", tag, (int)len, data);
+       else
+           rc = snprintf(buffer, sizeof(buffer), "%s", tag);
+       INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+       ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1582,19 +1576,15 @@ ctl_putunqstr(
        )
 {
        char buffer[512];
-       char *cp;
-       size_t tl;
-
-       tl = strlen(tag);
-       memcpy(buffer, tag, tl);
-       cp = buffer + tl;
-       if (len > 0) {
-               INSIST(tl + 1 + len <= sizeof(buffer));
-               *cp++ = '=';
-               memcpy(cp, data, len);
-               cp += len;
-       }
-       ctl_putdata(buffer, (u_int)(cp - buffer), 0);
+       int  rc;
+
+       INSIST(len < sizeof(buffer));
+       if (len)
+           rc = snprintf(buffer, sizeof(buffer), "%s=%.*s", tag, (int)len, data);
+       else
+           rc = snprintf(buffer, sizeof(buffer), "%s", tag);
+       INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+       ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 
@@ -1609,20 +1599,14 @@ ctl_putdblf(
        double          d
        )
 {
-       char *cp;
-       const char *cq;
        char buffer[200];
-
-       cp = buffer;
-       cq = tag;
-       while (*cq != '\0')
-               *cp++ = *cq++;
-       *cp++ = '=';
-       INSIST((size_t)(cp - buffer) < sizeof(buffer));
-       snprintf(cp, sizeof(buffer) - (cp - buffer), use_f ? "%.*f" : "%.*g",
-           precision, d);
-       cp += strlen(cp);
-       ctl_putdata(buffer, (unsigned)(cp - buffer), 0);
+       int  rc;
+       
+       rc = snprintf(buffer, sizeof(buffer),
+                     (use_f ? "%s=%.*f" : "%s=%.*g"),
+                     tag, precision, d);
+       INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+       ctl_putdata(buffer, (u_int)rc, 0);
 }
 
 /*
@@ -1634,20 +1618,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++ = '=';
-       INSIST((cp - buffer) < (int)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);
 }
 
 /*
@@ -1662,21 +1638,16 @@ ctl_putcal(
        )
 {
        char buffer[100];
-       unsigned numch;
-
-       numch = snprintf(buffer, sizeof(buffer),
-                       "%s=%04d%02d%02d%02d%02d",
-                       tag,
-                       pcal->year,
-                       pcal->month,
-                       pcal->monthday,
-                       pcal->hour,
-                       pcal->minute
-                       );
-       INSIST(numch < sizeof(buffer));
-       ctl_putdata(buffer, numch, 0);
+       int  rc;
 
-       return;
+       rc = snprintf(buffer, sizeof(buffer),
+                     "%s=%04d%02d%02d%02d%02d",
+                     tag,
+                     pcal->year, pcal->month, pcal->monthday,
+                     pcal->hour, pcal->minute
+               );
+       INSIST(rc >= 0 && (size_t)rc < sizeof(buffer));
+       ctl_putdata(buffer, (u_int)rc, 0);
 }
 #endif
 
@@ -1689,28 +1660,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;
-       INSIST((cp - buffer) < (int)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);
 }
 
 
@@ -1724,20 +1690,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++ = '=';
-       INSIST((cp - buffer) < (int)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);
 }
 
 
@@ -1750,20 +1708,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++ = '=';
-       INSIST((cp - buffer) < (int)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);
 }
 
 
@@ -1776,21 +1726,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++ = '=';
-       INSIST((size_t)(cp - buffer) < sizeof(buffer));
-       snprintf(cp, sizeof(buffer) - (cp - buffer), "0x%08x.%08x",
-                (u_int)ts->l_ui, (u_int)ts->l_uf);
-       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);
 }
 
 
@@ -1804,24 +1747,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);
-       INSIST((cp - buffer) < (int)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);
 }
 
 
@@ -1834,34 +1770,22 @@ ctl_putrefid(
        u_int32         refid
        )
 {
-       char    output[16];
-       char *  optr;
-       char *  oplim;
-       char *  iptr;
-       char *  iplim;
-       char *  past_eq = NULL;
-
-       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((int)*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);
 }
 
 
@@ -1875,26 +1799,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++;
-       *cp++ = '=';
+       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--;
-               INSIST((cp - buffer) < (int)sizeof(buffer));
-               snprintf(cp, sizeof(buffer) - (cp - buffer),
-                        " %.2f", arr[i] * 1e3);
-               cp += strlen(cp);
+               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, (unsigned)(cp - buffer), 0);
+       ctl_putdata(buffer, (u_int)(cp - buffer), 0);
 }
 
 /*