[Git][NTPsec/ntpsec][master] 2 commits: CVE-2016-7434: Crash from malformed mrulist request
Daniel Fox Franke
gitlab at mg.gitlab.com
Tue Nov 22 21:14:17 UTC 2016
Daniel Fox Franke pushed to branch master at NTPsec / ntpsec
Commits:
bdbc4cb5 by Daniel Fox Franke at 2016-11-22T16:12:57-05:00
CVE-2016-7434: Crash from malformed mrulist request
Patch ported from NTP Classic and mainly due to Juergen Perlinger
- - - - -
1fb1136a by Daniel Fox Franke at 2016-11-22T16:12:57-05:00
CVE-2016-7429: Avoid interface update on possibly-spoofed responses
Adapted from the NTP Classic patch by Juergen Perlinger
- - - - -
2 changed files:
- ntpd/ntp_control.c
- ntpd/ntp_peer.c
Changes:
=====================================
ntpd/ntp_control.c
=====================================
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -3286,15 +3286,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;
@@ -3302,7 +3304,7 @@ static void read_mru_list(
int mincount;
u_int maxlstint;
sockaddr_u laddr;
- endpt * lcladr;
+ endpt * lcladr;
u_int count;
u_int ui;
u_int uf;
@@ -3311,7 +3313,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;
@@ -3363,47 +3365,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 at 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)) {
- /* coverity[unchecked_value] */
- 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);
@@ -3523,7 +3546,6 @@ static void read_mru_list(
ctl_flushpkt(0);
}
-
/*
* Send a ifstats entry in response to a "ntpq -c ifstats" request.
*
=====================================
ntpd/ntp_peer.c
=====================================
--- a/ntpd/ntp_peer.c
+++ b/ntpd/ntp_peer.c
@@ -288,62 +288,44 @@ findpeer(
findpeer_calls++;
srcadr = &rbufp->recv_srcadr;
hash = NTP_HASH_ADDR(srcadr);
- for (p = peer_hash[hash]; p != NULL; p = p->adr_link) {
- if (ADDR_PORT_EQ(srcadr, &p->srcadr)) {
-
- /*
- * if the association matching rules determine
- * that this is not a valid combination, then
- * look for the next valid peer association.
- */
- *action = MATCH_ASSOC(p->hmode, pkt_mode);
-
- /*
- * A response to our manycastclient solicitation
- * might be misassociated with an ephemeral peer
- * already spun for the server. If the packet's
- * org timestamp doesn't match the peer's, check
- * if it matches the ACST prototype peer's. If
- * so it is a redundant solicitation response,
- * return AM_ERR to discard it. [Bug 1762]
- */
- if (MODE_SERVER == pkt_mode &&
- AM_PROCPKT == *action) {
- pkt = &rbufp->recv_pkt;
- NTOHL_FP(&pkt->org, &pkt_org);
- if (!L_ISEQU(&p->org, &pkt_org) &&
- findmanycastpeer(rbufp))
- *action = AM_ERR;
- }
-
- /*
- * if an error was returned, exit back right
- * here.
- */
- if (*action == AM_ERR)
- return NULL;
-
- /*
- * if a match is found, we stop our search.
- */
- if (*action != AM_NOMATCH)
- break;
- }
- }
+ for (p = peer_hash[hash]; p != NULL; p = p->adr_link) {
+ /* [Classic Bug 3072] ensure interface of peer matches */
+ if (p->dstadr != rbufp->dstadr) continue;
+
+ /* ensure peer source address matches */
+ if (!ADDR_PORT_EQ(srcadr, &p->srcadr)) continue;
+
+ /* If the association matching rules determine that this
+ * is not a valid combination, then look for the next
+ * valid peer association.
+ */
+ *action = MATCH_ASSOC(p->hmode, pkt_mode);
+
+ /* A response to our manycastclient solicitation might
+ * be misassociated with an ephemeral peer already spun
+ * for the server. If the packet's org timestamp
+ * doesn't match the peer's, check if it matches the
+ * ACST prototype peer's. If so it is a redundant
+ * solicitation response, return AM_ERR to discard it.
+ * [Bug 1762]
+ */
+ if (MODE_SERVER == pkt_mode && AM_PROCPKT == *action) {
+ pkt = &rbufp->recv_pkt;
+ NTOHL_FP(&pkt->org, &pkt_org);
+ if (!L_ISEQU(&p->org, &pkt_org) &&
+ findmanycastpeer(rbufp))
+ *action = AM_ERR;
+ }
+
+ /* If an error was returned, exit back right here. */
+ if (*action == AM_ERR) return NULL;
+
+ /* If a match is found, we stop our search. */
+ if (*action != AM_NOMATCH) break;
+ }
- /*
- * If no matching association is found
- */
- if (NULL == p) {
- *action = MATCH_ASSOC(NO_PEER, pkt_mode);
- } else if (p->dstadr != rbufp->dstadr) {
- set_peerdstadr(p, rbufp->dstadr);
- if (p->dstadr == rbufp->dstadr) {
- DPRINTF(1, ("Changed %s local address to match response\n",
- socktoa(&p->srcadr)));
- return findpeer(rbufp, pkt_mode, action);
- }
- }
+ /* If no matching association is found */
+ if (NULL == p) *action = MATCH_ASSOC(NO_PEER, pkt_mode);
return p;
}
@@ -554,7 +536,7 @@ set_peerdstadr(
{
struct peer * unlinked;
- if (p->dstadr == dstadr)
+ if (p == NULL || p->dstadr == dstadr)
return;
/*
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/b20dd545da6a0b39506c8f4cd179ee5595a978b2...1fb1136aa8964218719ed561bd608d418150c9a3
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ntpsec.org/pipermail/vc/attachments/20161122/526feb4e/attachment.html>
More information about the vc
mailing list