[Sec 3075] Core Dump. Added missing paramter validation in read_mru_list().

[Sec 3082] (title too long -- Variation of [Sec 3075].)
 - more hardening to read_mru_list().

bk: 577c2368SYIYqejizygC-8LgXTNcGQ

diff --git a/ChangeLog b/ChangeLog
index 0805467dc6b9b1ce7768a039f6a2d87af37546b9..e89f31a9aca59b22be31eb927f32520884356985 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+---
+* [Sec 3075] Malicious crafted read_mru_list() packet will crash ntpd.
+  - added missing paramter validation in read_mru_list(). perlinger@ntp.org
+* [Sec 3082] (title too long -- Variation of [Sec 3075].)
+  - more hardening to read_mru_list(). perlinger@ntp.org
+
 ---
 (4.2.8p8) 2016/06/02 Released by Harlan Stenn <stenn@ntp.org>
 
@@ -19,7 +25,7 @@
 * Fix typo in ntp-wait and plot_summary.  HStenn.
 * Make sure we have an "author" file for git imports.  HStenn.
 * Update the sntp problem tests for MacOS.  HStenn.
-  
+
 ---
 (4.2.8p7) 2016/04/26 Released by Harlan Stenn <stenn@ntp.org>
 

diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
index 07b5697f1536605efed3f4ee726d21ee94a59a70..b6b6006293e8cc576daf5b14f5d58d4dfc0b0bdd 100644 (file)
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -3954,15 +3954,17 @@ static void read_mru_list(
        int restrict_mask
        )
 {
-       const char              nonce_text[] =          "nonce";
-       const char              frags_text[] =          "frags";
-       const char              limit_text[] =          "limit";
-       const char              mincount_text[] =       "mincount";
-       const char              resall_text[] =         "resall";
-       const char              resany_text[] =         "resany";
-       const char              maxlstint_text[] =      "maxlstint";
-       const char              laddr_text[] =          "laddr";
-       const char              resaxx_fmt[] =          "0x%hx";
+       static const char       nulltxt[1] =            { '\0' };       
+       static const char       nonce_text[] =          "nonce";
+       static const char       frags_text[] =          "frags";
+       static const char       limit_text[] =          "limit";
+       static const char       mincount_text[] =       "mincount";
+       static const char       resall_text[] =         "resall";
+       static const char       resany_text[] =         "resany";
+       static const char       maxlstint_text[] =      "maxlstint";
+       static const char       laddr_text[] =          "laddr";
+       static const char       resaxx_fmt[] =          "0x%hx";
+       
        u_int                   limit;
        u_short                 frags;
        u_short                 resall;
@@ -3979,7 +3981,7 @@ static void read_mru_list(
        char                    buf[128];
        struct ctl_var *        in_parms;
        const struct ctl_var *  v;
-       char *                  val;
+       const char *            val;
        const char *            pch;
        char *                  pnonce;
        int                     nonce_valid;
@@ -4031,46 +4033,68 @@ static void read_mru_list(
        ZERO(last);
        ZERO(addr);
 
-       while (NULL != (v = ctl_getitem(in_parms, &val)) &&
+       /* have to go through '(void*)' to drop 'const' property from pointer.
+        * ctl_getitem()' needs some cleanup, too.... perlinger@ntp.org
+        */
+       while (NULL != (v = ctl_getitem(in_parms, (void*)&val)) &&
               !(EOV & v->flags)) {
                int si;
 
+               if (NULL == val)
+                       val = nulltxt;
+
                if (!strcmp(nonce_text, v->text)) {
-                       if (NULL != pnonce)
-                               free(pnonce);
-                       pnonce = estrdup(val);
+                       free(pnonce);
+                       pnonce = (*val) ? estrdup(val) : NULL;
                } else if (!strcmp(frags_text, v->text)) {
-                       sscanf(val, "%hu", &frags);
+                       if (1 != sscanf(val, "%hu", &frags))
+                               goto blooper;
                } else if (!strcmp(limit_text, v->text)) {
-                       sscanf(val, "%u", &limit);
+                       if (1 != sscanf(val, "%u", &limit))
+                               goto blooper;
                } else if (!strcmp(mincount_text, v->text)) {
-                       if (1 != sscanf(val, "%d", &mincount) ||
-                           mincount < 0)
+                       if (1 != sscanf(val, "%d", &mincount))
+                               goto blooper;
+                       if (mincount < 0)
                                mincount = 0;
                } else if (!strcmp(resall_text, v->text)) {
-                       sscanf(val, resaxx_fmt, &resall);
+                       if (1 != sscanf(val, resaxx_fmt, &resall))
+                               goto blooper;
                } else if (!strcmp(resany_text, v->text)) {
-                       sscanf(val, resaxx_fmt, &resany);
+                       if (1 != sscanf(val, resaxx_fmt, &resany))
+                               goto blooper;
                } else if (!strcmp(maxlstint_text, v->text)) {
-                       sscanf(val, "%u", &maxlstint);
+                       if (1 != sscanf(val, "%u", &maxlstint))
+                               goto blooper;
                } else if (!strcmp(laddr_text, v->text)) {
-                       if (decodenetnum(val, &laddr))
-                               lcladr = getinterface(&laddr, 0);
+                       if (!decodenetnum(val, &laddr))
+                               goto blooper;
+                       lcladr = getinterface(&laddr, 0);
                } else if (1 == sscanf(v->text, last_fmt, &si) &&
                           (size_t)si < COUNTOF(last)) {
-                       if (2 == sscanf(val, "0x%08x.%08x", &ui, &uf)) {
-                               last[si].l_ui = ui;
-                               last[si].l_uf = uf;
-                               if (!SOCK_UNSPEC(&addr[si]) &&
-                                   si == priors)
-                                       priors++;
-                       }
+                       if (2 != sscanf(val, "0x%08x.%08x", &ui, &uf))
+                               goto blooper;
+                       last[si].l_ui = ui;
+                       last[si].l_uf = uf;
+                       if (!SOCK_UNSPEC(&addr[si]) && si == priors)
+                               priors++;
                } else if (1 == sscanf(v->text, addr_fmt, &si) &&
                           (size_t)si < COUNTOF(addr)) {
-                       if (decodenetnum(val, &addr[si])
-                           && last[si].l_ui && last[si].l_uf &&
-                           si == priors)
+                       if (!decodenetnum(val, &addr[si]))
+                               goto blooper;
+                       if (last[si].l_ui && last[si].l_uf && si == priors)
                                priors++;
+               } else {
+                       DPRINTF(1, ("read_mru_list: invalid key item: '%s' (ignored)\n",
+                                   v->text));
+                       continue;
+
+               blooper:
+                       DPRINTF(1, ("read_mru_list: invalid param for '%s': '%s' (bailing)\n",
+                                   v->text, val));
+                       free(pnonce);
+                       pnonce = NULL;
+                       break;
                }
        }
        free_varlist(in_parms);