improve network message sanity checkings

3 files changed, 57 insertions, 38 deletions

diff --git a/src/network.c b/src/network.c
index d7ab415..fb6ea90 100644
--- a/src/network.c
+++ b/src/network.c
@@ -171,30 +171,6 @@ void build_netmsg(struct nf_conntrack *ct, int query, struct nethdr *net)
 	build_netpld(ct, pld, query);
 }
 
-int handle_netmsg(struct nethdr *net)
-{
-	struct netpld *pld = NETHDR_DATA(net);
-
-	/* message too small: no room for the header */
-	if (ntohs(net->len) < NETHDR_ACK_SIZ)
-		return -1;
-
-	HDR_NETWORK2HOST(net);
-
-	if (IS_CTL(net))
-		return 0;
-
-	/* information received is too small */
-	if (net->len < sizeof(struct netpld))
-		return -1;
-
-	/* size mismatch! */
-	if (net->len < ntohs(pld->len) + NETHDR_SIZ)
-		return -1;
-
-	return 0;
-}
-
 static int local_seq_set = 0;
 
 /* this function only tracks, it does not update the last sequence received */
diff --git a/src/parse.c b/src/parse.c
index 8ef2e8d..645420d 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -20,6 +20,10 @@
 
 #include <libnetfilter_conntrack/libnetfilter_conntrack.h>
 
+#ifndef ssizeof
+#define ssizeof(x) (int)sizeof(x)
+#endif
+
 static void parse_u8(struct nf_conntrack *ct, int attr, void *data)
 {
 	uint8_t *value = (uint8_t *) data;
@@ -77,20 +81,40 @@ static parse h[ATTR_MAX] = {
 	[ATTR_REPL_NAT_SEQ_OFFSET_AFTER]	= parse_u32,
 };
 
-void parse_netpld(struct nf_conntrack *ct, struct netpld *pld, int *query)
+int
+parse_netpld(struct nf_conntrack *ct,
+	     struct nethdr *net,
+	     int *query,
+	     size_t remain)
 {
 	int len;
 	struct netattr *attr;
+	struct netpld *pld;
+
+	if (remain < NETHDR_SIZ + sizeof(struct netpld))
+		return -1;
+
+	pld = NETHDR_DATA(net);
+
+	if (remain < NETHDR_SIZ + sizeof(struct netpld) + ntohs(pld->len))
+		return -1;
+
+	if (net->len < NETHDR_SIZ + sizeof(struct netpld) + ntohs(pld->len))
+		return -1;
 
 	PLD_NETWORK2HOST(pld);
 	len = pld->len;
 	attr = PLD_DATA(pld);
 
-	while (len > 0) {
+	while (len > ssizeof(struct netattr)) {
 		ATTR_NETWORK2HOST(attr);
-		h[attr->nta_attr](ct, attr->nta_attr, NTA_DATA(attr));
+		if (attr->nta_len > len)
+			return -1;
+		if (h[attr->nta_attr])
+			h[attr->nta_attr](ct, attr->nta_attr, NTA_DATA(attr));
 		attr = NTA_NEXT(attr, len);
 	}
 
 	*query = pld->query;
+	return 0;
 }
diff --git a/src/sync-mode.c b/src/sync-mode.c
index a952a5b..7d73e2f 100644
--- a/src/sync-mode.c
+++ b/src/sync-mode.c
@@ -34,10 +34,9 @@
 #include <string.h>
 #include <stdlib.h>
 
-static void do_mcast_handler_step(struct nethdr *net)
+static void do_mcast_handler_step(struct nethdr *net, size_t remain)
 {
 	int query;
-	struct netpld *pld = NETHDR_DATA(net);
 	char __ct[nfct_maxsize()];
 	struct nf_conntrack *ct = (struct nf_conntrack *)(void*) __ct;
 	struct us_conntrack *u;
@@ -57,8 +56,11 @@ static void do_mcast_handler_step(struct nethdr *net)
 
 	memset(ct, 0, sizeof(__ct));
 
-	/* XXX: check for malformed */
-	parse_netpld(ct, pld, &query);
+	if (parse_netpld(ct, net, &query, remain) == -1) {
+		STATE(malformed)++;
+		dlog(LOG_ERR, "parsing failed: malformed message");
+		return;
+	}
 
 	switch(query) {
 	case NFCT_Q_CREATE:
@@ -91,6 +93,7 @@ retry:
 			debug_ct(ct, "can't destroy");
 		break;
 	default:
+		STATE(malformed)++;
 		dlog(LOG_ERR, "mcast unknown query %d\n", query);
 		break;
 	}
@@ -113,24 +116,40 @@ static void mcast_handler(void)
 
 		if (remain < NETHDR_SIZ) {
 			STATE(malformed)++;
+			dlog(LOG_WARNING, "no room for header");
 			break;
 		}
 
 		if (ntohs(net->len) > remain) {
-			dlog(LOG_ERR, "fragmented messages");
+			STATE(malformed)++;
+			dlog(LOG_WARNING, "fragmented message");
 			break;
 		}
 
+		if (IS_CTL(net)) {
+			if (remain < NETHDR_ACK_SIZ) {
+				STATE(malformed)++;
+				dlog(LOG_WARNING, "no room for ctl message");
+			}
+
+			if (ntohs(net->len) < NETHDR_ACK_SIZ) {
+				STATE(malformed)++;
+				dlog(LOG_WARNING, "ctl header too small");
+			}
+		} else {
+			if (ntohs(net->len) < NETHDR_SIZ) {
+				STATE(malformed)++;
+				dlog(LOG_WARNING, "header too small");
+			}
+		}
+
 		debug("recv sq: %u fl:%u len:%u (rem:%d)\n", 
 			ntohl(net->seq), ntohs(net->flags),
 			ntohs(net->len), remain);
 
-		/* sanity check and convert nethdr to host byte order */
-		if (handle_netmsg(net) == -1) {
-			STATE(malformed)++;
-			return;
-		}
-		do_mcast_handler_step(net);
+		HDR_NETWORK2HOST(net);
+
+		do_mcast_handler_step(net, remain);
 		ptr += net->len;
 		remain -= net->len;
 	}