From: Juergen Perlinger <perlinger@ntp.org>
Date: Fri, 19 Feb 2016 21:42:25 +0000 (+0100)
Subject: [Bug 3010] remote configuration trustedkey/requestkey values are not properly validated
X-Git-Tag: NTP_4_2_8P7~22^2^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5448cfca39af51fc2d53b0c86c6828ff320af99f;p=thirdparty%2Fntp.git

[Bug 3010] remote configuration trustedkey/requestkey values are not properly validated

bk: 56c78c41-oKNCUhyU5kKQCxLjnp0Fw
---

diff --git a/ChangeLog b/ChangeLog
index c70fe8fc5..d5a2f9a23 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,8 @@
 
 * [Bug 2994] Systems with HAVE_SIGNALED_IO fail to compile. perlinger@ntp.org
 * [Bug 2995] Fixes to compile on Windows
+* [Bug 3010] remote configuration trustedkey/requestkey values
+  are not properly validated. perlinger@ntp.org
 
 ---
 (4.2.8p6) 2016/01/20 Released by Harlan Stenn <stenn@ntp.org>
diff --git a/ntpd/ntp_request.c b/ntpd/ntp_request.c
index ba968e2c8..4641da2ad 100644
--- a/ntpd/ntp_request.c
+++ b/ntpd/ntp_request.c
@@ -2297,34 +2297,62 @@ do_setclr_trap(
 	return;
 }
 
-
-
 /*
- * set_request_keyid - set the keyid used to authenticate requests
+ * Validate a request packet for a new request or control key:
+ *  - only one item allowed
+ *  - key must be valid (that is, known, and not in the autokey range)
  */
 static void
-set_request_keyid(
-	sockaddr_u *srcadr,
-	endpt *inter,
+set_keyid_checked(
+	keyid_t        *into,
+	const char     *what,
+	sockaddr_u     *srcadr,
+	endpt          *inter,
 	struct req_pkt *inpkt
 	)
 {
 	keyid_t *pkeyid;
+	keyid_t  tmpkey;
 
-	/*
-	 * Restrict ourselves to one item only.
-	 */
+	/* restrict ourselves to one item only */
 	if (INFO_NITEMS(inpkt->err_nitems) > 1) {
-		msyslog(LOG_ERR, "set_request_keyid: err_nitems > 1");
+		msyslog(LOG_ERR, "set_keyid_checked[%s]: err_nitems > 1",
+			what);
 		req_ack(srcadr, inter, inpkt, INFO_ERR_FMT);
 		return;
 	}
 
+	/* plug the new key from the packet */
 	pkeyid = (keyid_t *)&inpkt->u;
-	info_auth_keyid = ntohl(*pkeyid);
+	tmpkey = ntohl(*pkeyid);
+
+	/* validate the new key id, claim data error on failure */
+	if (tmpkey < 1 || tmpkey > NTP_MAXKEY || !auth_havekey(tmpkey)) {
+		msyslog(LOG_ERR, "set_keyid_checked[%s]: invalid key id: %ld",
+			what, (long)tmpkey);
+		req_ack(srcadr, inter, inpkt, INFO_ERR_NODATA);
+		return;
+	}
+
+	/* if we arrive here, the key is good -- use it */
+	*into = tmpkey;
 	req_ack(srcadr, inter, inpkt, INFO_OKAY);
 }
 
+/*
+ * set_request_keyid - set the keyid used to authenticate requests
+ */
+static void
+set_request_keyid(
+	sockaddr_u *srcadr,
+	endpt *inter,
+	struct req_pkt *inpkt
+	)
+{
+	set_keyid_checked(&info_auth_keyid, "request",
+			  srcadr, inter, inpkt);
+}
+
 
 
 /*
@@ -2337,20 +2365,8 @@ set_control_keyid(
 	struct req_pkt *inpkt
 	)
 {
-	keyid_t *pkeyid;
-
-	/*
-	 * Restrict ourselves to one item only.
-	 */
-	if (INFO_NITEMS(inpkt->err_nitems) > 1) {
-		msyslog(LOG_ERR, "set_control_keyid: err_nitems > 1");
-		req_ack(srcadr, inter, inpkt, INFO_ERR_FMT);
-		return;
-	}
-
-	pkeyid = (keyid_t *)&inpkt->u;
-	ctl_auth_keyid = ntohl(*pkeyid);
-	req_ack(srcadr, inter, inpkt, INFO_OKAY);
+	set_keyid_checked(&ctl_auth_keyid, "control",
+			  srcadr, inter, inpkt);
 }