[Sec 3414] ntpq: decodearr() can write beyond its 'buf' limits

bk: 59600eacHNF0J6NIJgOKyMHSraP_WQ

diff --git a/ChangeLog b/ChangeLog
index a1a1cfae45a59975bb12958d206f6c0a48e631b5..52248958bab2b7125017d3c6f012624dacb33a7c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+---
+* [Sec 3414] ntpq: decodearr() can write beyond its 'buf' limits
+ - initial patch by <stenn@ntp.org>, extended by <perlinger@ntp.org>
+
 ---
 (4.2.8p10-win-beta1) 2017/03/21 Released by Harlan Stenn <stenn@ntp.org>
 (4.2.8p10)

diff --git a/ntpq/ntpq.c b/ntpq/ntpq.c
index 547f3cc5d22bfb52805b51d1f30e5481168b928c..461611462021ef6787fe8ec0a926aef4e8f278d9 100644 (file)
--- a/ntpq/ntpq.c
+++ b/ntpq/ntpq.c
@@ -189,7 +189,7 @@ static      int     getarg          (const char *, int, arg_v *);
 static int     findcmd         (const char *, struct xcmd *,
                                 struct xcmd *, struct xcmd **);
 static int     rtdatetolfp     (char *, l_fp *);
-static int     decodearr       (char *, int *, l_fp *);
+static int     decodearr       (char *, int *, l_fp *, int);
 static void    help            (struct parse *, FILE *);
 static int     helpsort        (const void *, const void *);
 static void    printusage      (struct xcmd *, FILE *);
@@ -227,6 +227,16 @@ static     void    on_ctrlc        (void);
 static int     my_easprintf    (char**, const char *, ...) NTP_PRINTF(2, 3);
 void   ntpq_custom_opt_handler (tOptions *, tOptDesc *);
 
+/* read a character from memory and expand to integer */
+static inline int
+pgetc(
+       const char *cp
+       )
+{
+       return (int)*(const unsigned char*)cp;
+}
+
+
 #ifdef OPENSSL
 # ifdef HAVE_EVP_MD_DO_ALL_SORTED
 static void list_md_fn(const EVP_MD *m, const char *from,
@@ -2014,7 +2024,7 @@ rtdatetolfp(
         * d[d]-Mth-y[y[y[y]]] hh:mm:ss
         */
        cp = str;
-       if (!isdigit((int)*cp)) {
+       if (!isdigit(pgetc(cp))) {
                if (*cp == '-') {
                        /*
                         * Catch special case
@@ -2026,7 +2036,7 @@ rtdatetolfp(
        }
 
        cal.monthday = (u_char) (*cp++ - '0');  /* ascii dependent */
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.monthday = (u_char)((cal.monthday << 3) + (cal.monthday << 1));
                cal.monthday = (u_char)(cal.monthday + *cp++ - '0');
        }
@@ -2048,18 +2058,18 @@ rtdatetolfp(
        if (*cp++ != '-')
            return 0;
 
-       if (!isdigit((int)*cp))
+       if (!isdigit(pgetc(cp)))
            return 0;
        cal.year = (u_short)(*cp++ - '0');
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.year = (u_short)((cal.year << 3) + (cal.year << 1));
                cal.year = (u_short)(*cp++ - '0');
        }
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.year = (u_short)((cal.year << 3) + (cal.year << 1));
                cal.year = (u_short)(cal.year + *cp++ - '0');
        }
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.year = (u_short)((cal.year << 3) + (cal.year << 1));
                cal.year = (u_short)(cal.year + *cp++ - '0');
        }
@@ -2072,26 +2082,26 @@ rtdatetolfp(
                return 1;
        }
 
-       if (*cp++ != ' ' || !isdigit((int)*cp))
+       if (*cp++ != ' ' || !isdigit(pgetc(cp)))
            return 0;
        cal.hour = (u_char)(*cp++ - '0');
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.hour = (u_char)((cal.hour << 3) + (cal.hour << 1));
                cal.hour = (u_char)(cal.hour + *cp++ - '0');
        }
 
-       if (*cp++ != ':' || !isdigit((int)*cp))
+       if (*cp++ != ':' || !isdigit(pgetc(cp)))
            return 0;
        cal.minute = (u_char)(*cp++ - '0');
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.minute = (u_char)((cal.minute << 3) + (cal.minute << 1));
                cal.minute = (u_char)(cal.minute + *cp++ - '0');
        }
 
-       if (*cp++ != ':' || !isdigit((int)*cp))
+       if (*cp++ != ':' || !isdigit(pgetc(cp)))
            return 0;
        cal.second = (u_char)(*cp++ - '0');
-       if (isdigit((int)*cp)) {
+       if (isdigit(pgetc(cp))) {
                cal.second = (u_char)((cal.second << 3) + (cal.second << 1));
                cal.second = (u_char)(cal.second + *cp++ - '0');
        }
@@ -2215,34 +2225,36 @@ decodeuint(
  */
 static int
 decodearr(
-       char *str,
-       int *narr,
-       l_fp *lfparr
+       char *cp,
+       int  *narr,
+       l_fp *lfpa,
+       int   amax
        )
 {
-       register char *cp, *bp;
-       register l_fp *lfp;
+       char *bp;
        char buf[60];
 
-       lfp = lfparr;
-       cp = str;
        *narr = 0;
 
-       while (*narr < 8) {
-               while (isspace((int)*cp))
-                   cp++;
-               if (*cp == '\0')
-                   break;
-
-               bp = buf;
-               while (!isspace((int)*cp) && *cp != '\0')
-                   *bp++ = *cp++;
-               *bp++ = '\0';
+       while (*narr < amax && *cp) {
+               if (isspace(pgetc(cp))) {
+                       do
+                               ++cp;
+                       while (*cp && isspace(pgetc(cp)));
+               } else {
+                       bp = buf;
+                       do {
+                               if (bp != (buf + sizeof(buf) - 1))
+                                       *bp++ = *cp;
+                               ++cp;
+                       } while (*cp && !isspace(pgetc(cp)));
+                       *bp = '\0';
 
-               if (!decodetime(buf, lfp))
-                   return 0;
-               (*narr)++;
-               lfp++;
+                       if (!decodetime(buf, lfpa))
+                               return 0;
+                       ++(*narr);
+                       ++lfpa;
+               }
        }
        return 1;
 }
@@ -3049,7 +3061,7 @@ nextvar(
        /*
         * Space past commas and white space
         */
-       while (cp < cpend && (*cp == ',' || isspace((int)*cp)))
+       while (cp < cpend && (*cp == ',' || isspace(pgetc(cp))))
                cp++;
        if (cp >= cpend)
                return 0;
@@ -3061,7 +3073,7 @@ nextvar(
        srclen = strcspn(cp, ",=\r\n");
        srclen = min(srclen, (size_t)(cpend - cp));
        len = srclen;
-       while (len > 0 && isspace((unsigned char)cp[len - 1]))
+       while (len > 0 && isspace(pgetc(&cp[len - 1])))
                len--;
        if (len >= sizeof(name))
            return 0;
@@ -3087,7 +3099,7 @@ nextvar(
         * So far, so good.  Copy out the value
         */
        cp++;   /* past '=' */
-       while (cp < cpend && (isspace((unsigned char)*cp) && *cp != '\r' && *cp != '\n'))
+       while (cp < cpend && (isspace(pgetc(cp)) && *cp != '\r' && *cp != '\n'))
                cp++;
        np = cp;
        if ('"' == *np) {
@@ -3108,7 +3120,7 @@ nextvar(
        /*
         * Trim off any trailing whitespace
         */
-       while (len > 0 && isspace((unsigned char)value[len - 1]))
+       while (len > 0 && isspace(pgetc(&value[len - 1])))
                len--;
        value[len] = '\0';
 
@@ -3191,7 +3203,7 @@ rawprint(
                         */
                        if (cp == (cpend - 1) || *(cp + 1) != '\n')
                            makeascii(1, cp, fp);
-               } else if (isspace((unsigned char)*cp) || isprint((unsigned char)*cp))
+               } else if (isspace(pgetc(cp)) || isprint(pgetc(cp)))
                        putc(*cp, fp);
                else
                        makeascii(1, cp, fp);
@@ -3399,7 +3411,7 @@ cookedprint(
                        break;
 
                case TS:
-                       if (!decodets(value, &lfp))
+                       if (!value || !decodets(value, &lfp))
                                output_raw = '?';
                        else
                                output(fp, name, prettydate(&lfp));
@@ -3407,7 +3419,7 @@ cookedprint(
 
                case HA:        /* fallthru */
                case NA:
-                       if (!decodenetnum(value, &hval)) {
+                       if (!value || !decodenetnum(value, &hval)) {
                                output_raw = '?';
                        } else if (fmt == HA){
                                output(fp, name, nntohost(&hval));
@@ -3417,7 +3429,9 @@ cookedprint(
                        break;
 
                case RF:
-                       if (decodenetnum(value, &hval)) {
+                       if (!value) {
+                               output_raw = '?';
+                       } else if (decodenetnum(value, &hval)) {
                                if (ISREFCLOCKADR(&hval))
                                        output(fp, name,
                                               refnumtoa(&hval));
@@ -3431,7 +3445,7 @@ cookedprint(
                        break;
 
                case LP:
-                       if (!decodeuint(value, &uval) || uval > 3) {
+                       if (!value || !decodeuint(value, &uval) || uval > 3) {
                                output_raw = '?';
                        } else {
                                b[0] = (0x2 & uval)
@@ -3446,7 +3460,7 @@ cookedprint(
                        break;
 
                case OC:
-                       if (!decodeuint(value, &uval)) {
+                       if (!value || !decodeuint(value, &uval)) {
                                output_raw = '?';
                        } else {
                                snprintf(b, sizeof(b), "%03lo", uval);
@@ -3455,14 +3469,14 @@ cookedprint(
                        break;
 
                case AR:
-                       if (!decodearr(value, &narr, lfparr))
+                       if (!value || !decodearr(value, &narr, lfparr, 8))
                                output_raw = '?';
                        else
                                outputarr(fp, name, narr, lfparr);
                        break;
 
                case FX:
-                       if (!decodeuint(value, &uval))
+                       if (!value || !decodeuint(value, &uval))
                                output_raw = '?';
                        else
                                output(fp, name, tstflags(uval));
@@ -3611,8 +3625,8 @@ static void list_md_fn(const EVP_MD *m, const char *from, const char *to, void *
     /* Lowercase names aren't accepted by keytype_from_text in ssl_init.c */
 
     for( cp = name; *cp; cp++ ) {
-       if( islower((unsigned char)*cp) )
-           return;
+           if( islower(pgetc(cp)) )
+                   return;
     }
     len = (cp - name) + 1;