[Git][NTPsec/ntpsec][master] Perform all bogon checks before updating peer variables (CVE-2016-4954)

[Daniel Fox Franke]

Daniel Fox Franke pushed to branch master at NTPsec / ntpsec


Commits:
28cf4470 by Daniel Fox Franke at 2016-06-02T12:18:35-04:00
Perform all bogon checks before updating peer variables (CVE-2016-4954)

NTP Classic bug #3044. Although NTP Classic assigned this a CVE, I
don't regard it a security issue because the tardy checks aren't
relevant to authentication. Failing to sanity-check the leap, stratum,
and root dispersion fields doesn't make it any easier for an adversary
to spoof responses, and it doesn't enable an authentic-but-malicious
server to put the client into any state worse than one that it could
put it into by supplying values that pass these checks.

- - - - -


1 changed file:

- ntpd/ntp_proto.c


Changes:

=====================================
ntpd/ntp_proto.c
=====================================
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -1237,8 +1237,6 @@ process_packet(
 
 	UNUSED_ARG(len);
 
-	sys_processed++;
-	peer->processed++;
 	p_del = FPTOD(NTOHS_FP(pkt->rootdelay));
 	p_offset = 0;
 	p_disp = FPTOD(NTOHS_FP(pkt->rootdisp));
@@ -1250,6 +1248,35 @@ process_packet(
 	pleap = PKT_LEAP(pkt->li_vn_mode);
 	pversion = PKT_VERSION(pkt->li_vn_mode);
 	pstratum = PKT_TO_STRATUM(pkt->stratum);
+
+	/*
+	 * Verify the server is synchronized; that is, the leap bits,
+	 * stratum and root distance are valid.
+	 */
+	if (pleap == LEAP_NOTINSYNC ||		/* test 6 */
+	    pstratum < sys_floor || pstratum >= sys_ceiling)
+		peer->flash |= BOGON6;		/* bad synch or strat */
+	if (p_del / 2 + p_disp >= MAXDISPERSE)	/* test 7 */
+		peer->flash |= BOGON7;		/* bad header */
+
+	/*
+	 * If any tests fail at this point, the packet is discarded.
+	 * Note that some flashers may have already been set in the
+	 * receive() routine.
+	 */
+	if (peer->flash & PKT_BOGON_MASK) {
+		peer->seldisptoolarge++;
+#ifdef DEBUG
+		if (debug)
+			printf("packet: flash header %04x\n",
+			    peer->flash);
+#endif
+		return;
+	}
+
+	sys_processed++;
+	peer->processed++;
+
 	if (peer->outcount) peer->outcount--;  /* dup, peer with shorter poll */
 
 	/*
@@ -1288,31 +1315,6 @@ process_packet(
 	poll_update(peer, peer->hpoll);
 
 	/*
-	 * Verify the server is synchronized; that is, the leap bits,
-	 * stratum and root distance are valid.
-	 */
-	if (pleap == LEAP_NOTINSYNC ||		/* test 6 */
-	    pstratum < sys_floor || pstratum >= sys_ceiling)
-		peer->flash |= BOGON6;		/* bad synch or strat */
-	if (p_del / 2 + p_disp >= MAXDISPERSE)	/* test 7 */
-		peer->flash |= BOGON7;		/* bad header */
-
-	/*
-	 * If any tests fail at this point, the packet is discarded.
-	 * Note that some flashers may have already been set in the
-	 * receive() routine.
-	 */
-	if (peer->flash & PKT_BOGON_MASK) {
-		peer->seldisptoolarge++;
-#ifdef DEBUG
-		if (debug)
-			printf("packet: flash header %04x\n",
-			    peer->flash);
-#endif
-		return;
-	}
-
-	/*
 	 * If the peer was previously unreachable, raise a trap. In any
 	 * case, mark it reachable.
 	 */



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/28cf44704a3496a18e6f9bd9a2260efbf0f66892
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ntpsec.org/pipermail/vc/attachments/20160602/fd6b35da/attachment.html>