From: Juergen Perlinger <perlinger@ntp.org>
Date: Wed, 6 Aug 2014 20:22:18 +0000 (+0200)
Subject: Work around Coverity issue over mismatch of formal and paket layout.
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d720a9eb90ef4d0bbd783319ac9d30edee7afa3d;p=thirdparty%2Fntp.git

Work around Coverity issue over mismatch of formal and paket layout.

bk: 53e28e7abivJnio2A9SGuIJVTWaGWg
---

diff --git a/sntp/networking.c b/sntp/networking.c
index 46c008dfd7..3a6bb8e861 100644
--- a/sntp/networking.c
+++ b/sntp/networking.c
@@ -62,6 +62,30 @@ recvdata(
 	return recvc;
 }
 
+/* Parsing from a short 'struct pkt' directly is bound to create
+ * coverity warnings. These are hard to avoid, as the formal declaration
+ * does not reflect the true layout in the presence of autokey extension
+ * fields. Parsing and skipping the extension fields od a received paket
+ * until there's only the MAC left is better done in this separate
+ * function.
+ */
+static void*
+skip_efields(
+	u_int32 *head,	/* head of extension chain 	*/
+	u_int32 *tail	/* tail/end of extension chain	*/
+	)
+{
+	
+	u_int nlen;	/* next extension length */
+	while ((tail - head) > 6) {
+		nlen = ntohl(*head++) & 0xffff;
+		nlen = (nlen + 3) >> 2;
+		if (nlen > (tail - head) || nlen < 4)
+			return NULL;	/* Blooper! Inconsistent! */
+		head += nlen;
+	}
+	return head;
+}
 
 /*
 ** Check if it's data for us and whether it's useable or not.
@@ -82,16 +106,15 @@ process_pkt (
 	u_int		key_id;
 	struct key *	pkt_key;
 	int		is_authentic;
-	u_int		exten_words;
-	u_int		exten_words_used;
 	int		mac_size;
 	u_int		exten_len;
+	u_int32 *       exten_end;
+	u_int32 *       paket_end;
 	l_fp		sent_xmt;
 	l_fp		resp_org;
 
 	key_id = 0;
 	pkt_key = NULL;
-	exten_words_used = 0;
 	is_authentic = (HAVE_OPT(AUTHENTICATION)) ? 0 : -1;
 
 	/*
@@ -113,35 +136,36 @@ unusable:
 			func_name, pkt_len);
 		return PACKET_UNUSEABLE;
 	}
-	/* skip past the extensions, if any */
-	exten_words = ((unsigned)pkt_len - LEN_PKT_NOMAC) >> 2;
-	while (exten_words > 6) {
-		exten_len = ntohl(rpkt->exten[exten_words_used]) & 0xffff;
-		exten_len = (exten_len + 7) >> 2; /* convert to words, add 1 */
-		if (exten_len > exten_words || exten_len < 5)
-			goto unusable;
-		exten_words -= exten_len;
-		exten_words_used += exten_len;
-	}
+	/* Note: pkt_len is a multiple of 4 at this point! */
+	paket_end = (u_int32*)((char*)rpkt + pkt_len);
+	exten_end = skip_efields(rpkt->exten, paket_end);
+	if (NULL == exten_end)
+		goto unusable;
+	/* get size of MAC in cells; can be zero */
+	exten_len = (u_int)(paket_end - exten_end);
 
-	switch (exten_words) {
+	/* deduce action required from remaining length */
+	switch (exten_len) {
 
-	case 0:
+	case 0:	/* no MAC at all */
 		break;
 
-	case 1:
-		key_id = ntohl(rpkt->exten[exten_words_used]);
+	case 1:	/* crypto NAK */		
+		key_id = ntohl(*exten_end);
 		printf("Crypto NAK = 0x%08x\n", key_id);
 		break;
 
-	case 5:
-	case 6:
+	case 3: /* key ID + 3DES MAC -- unsupported! */
+		goto unusable;
+
+	case 5:	/* key ID + MD5 MAC */
+	case 6:	/* key ID + SHA MAC */
 		/*
 		** Look for the key used by the server in the specified
 		** keyfile and if existent, fetch it or else leave the
 		** pointer untouched
 		*/
-		key_id = ntohl(rpkt->exten[exten_words_used]);
+		key_id = ntohl(*exten_end);
 		get_key(key_id, &pkt_key);
 		if (!pkt_key) {
 			printf("unrecognized key ID = 0x%08x\n", key_id);
@@ -153,7 +177,7 @@ unusable:
 		** Generate a md5sum of the packet with the key from our
 		** keyfile and compare those md5sums.
 		*/
-		mac_size = exten_words << 2;
+		mac_size = exten_len << 2;
 		if (!auth_md5((char *)rpkt, pkt_len - mac_size,
 			      mac_size - 4, pkt_key)) {
 			is_authentic = FALSE;
@@ -167,7 +191,6 @@ unusable:
 
 	default:
 		goto unusable;
-		break;
 	}
 
 	switch (is_authentic) {