ospfd: fix SA warnings in ospfd, ospfclient

Fix some SA warnings in ospf GR and ospfclient code.

Signed-off-by: Mark Stapp <mjs@voltanet.io>

2 files changed, 96 insertions, 9 deletions

diff --git a/ospfclient/ospf_apiclient.c b/ospfclient/ospf_apiclient.c
index fb8ad3e60..d4f0dc953 100644
--- a/ospfclient/ospf_apiclient.c
+++ b/ospfclient/ospf_apiclient.c
@@ -565,6 +565,7 @@ static void ospf_apiclient_handle_lsa_update(struct ospf_apiclient *oclient,
 {
 	struct msg_lsa_change_notify *cn;
 	struct lsa_header *lsa;
+	void *p;
 	uint16_t lsalen;
 
 	cn = (struct msg_lsa_change_notify *)STREAM_DATA(msg->s);
@@ -578,9 +579,11 @@ static void ospf_apiclient_handle_lsa_update(struct ospf_apiclient *oclient,
 			__func__, lsalen, OSPF_MAX_LSA_SIZE);
 		return;
 	}
-	lsa = XMALLOC(MTYPE_OSPF_APICLIENT, lsalen);
 
-	memcpy(lsa, &(cn->data), lsalen);
+	p = XMALLOC(MTYPE_OSPF_APICLIENT, lsalen);
+
+	memcpy(p, &(cn->data), lsalen);
+	lsa = p;
 
 	/* Invoke registered update callback function */
 	if (oclient->update_notify) {
@@ -589,7 +592,7 @@ static void ospf_apiclient_handle_lsa_update(struct ospf_apiclient *oclient,
 	}
 
 	/* free memory allocated by ospf apiclient library */
-	XFREE(MTYPE_OSPF_APICLIENT, lsa);
+	XFREE(MTYPE_OSPF_APICLIENT, p);
 }
 
 static void ospf_apiclient_handle_lsa_delete(struct ospf_apiclient *oclient,
@@ -597,6 +600,7 @@ static void ospf_apiclient_handle_lsa_delete(struct ospf_apiclient *oclient,
 {
 	struct msg_lsa_change_notify *cn;
 	struct lsa_header *lsa;
+	void *p;
 	uint16_t lsalen;
 
 	cn = (struct msg_lsa_change_notify *)STREAM_DATA(msg->s);
@@ -610,9 +614,11 @@ static void ospf_apiclient_handle_lsa_delete(struct ospf_apiclient *oclient,
 			__func__, lsalen, OSPF_MAX_LSA_SIZE);
 		return;
 	}
-	lsa = XMALLOC(MTYPE_OSPF_APICLIENT, lsalen);
 
-	memcpy(lsa, &(cn->data), lsalen);
+	p = XMALLOC(MTYPE_OSPF_APICLIENT, lsalen);
+
+	memcpy(p, &(cn->data), lsalen);
+	lsa = p;
 
 	/* Invoke registered update callback function */
 	if (oclient->delete_notify) {
@@ -621,7 +627,7 @@ static void ospf_apiclient_handle_lsa_delete(struct ospf_apiclient *oclient,
 	}
 
 	/* free memory allocated by ospf apiclient library */
-	XFREE(MTYPE_OSPF_APICLIENT, lsa);
+	XFREE(MTYPE_OSPF_APICLIENT, p);
 }
 
 static void ospf_apiclient_msghandle(struct ospf_apiclient *oclient,
diff --git a/ospfd/ospf_gr_helper.c b/ospfd/ospf_gr_helper.c
index a7b20d1f0..bf6a45bcd 100644
--- a/ospfd/ospf_gr_helper.c
+++ b/ospfd/ospf_gr_helper.c
@@ -200,12 +200,38 @@ static int ospf_extract_grace_lsa_fields(struct ospf_lsa *lsa,
 
 	lsah = (struct lsa_header *)lsa->data;
 
-	length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE;
+	length = ntohs(lsah->length);
+
+	/* Check LSA len */
+	if (length <= OSPF_LSA_HEADER_SIZE) {
+		if (IS_DEBUG_OSPF_GR_HELPER)
+			zlog_debug("%s: Malformed packet: Invalid LSA len:%d",
+				   __func__, length);
+		return OSPF_GR_FAILURE;
+	}
+
+	length -= OSPF_LSA_HEADER_SIZE;
 
 	for (tlvh = TLV_HDR_TOP(lsah); sum < length;
 	     tlvh = TLV_HDR_NEXT(tlvh)) {
+
+		/* Check TLV len against overall LSA */
+		if (sum + TLV_SIZE(tlvh) > length) {
+			if (IS_DEBUG_OSPF_GR_HELPER)
+				zlog_debug("%s: Malformed packet: Invalid TLV len:%zu",
+					   __func__, TLV_SIZE(tlvh));
+			return OSPF_GR_FAILURE;
+		}
+
 		switch (ntohs(tlvh->type)) {
 		case GRACE_PERIOD_TYPE:
+			if (TLV_SIZE(tlvh) <
+			    sizeof(struct grace_tlv_graceperiod)) {
+				zlog_debug("%s: Malformed packet: Invalid grace TLV len:%zu",
+					   __func__, TLV_SIZE(tlvh));
+				return OSPF_GR_FAILURE;
+			}
+
 			grace_period = (struct grace_tlv_graceperiod *)tlvh;
 			*interval = ntohl(grace_period->interval);
 			sum += TLV_SIZE(tlvh);
@@ -216,6 +242,13 @@ static int ospf_extract_grace_lsa_fields(struct ospf_lsa *lsa,
 				return OSPF_GR_FAILURE;
 			break;
 		case RESTART_REASON_TYPE:
+			if (TLV_SIZE(tlvh) <
+			    sizeof(struct grace_tlv_restart_reason)) {
+				zlog_debug("%s: Malformed packet: Invalid reason TLV len:%zu",
+					   __func__, TLV_SIZE(tlvh));
+				return OSPF_GR_FAILURE;
+			}
+
 			gr_reason = (struct grace_tlv_restart_reason *)tlvh;
 			*reason = gr_reason->reason;
 			sum += TLV_SIZE(tlvh);
@@ -224,6 +257,13 @@ static int ospf_extract_grace_lsa_fields(struct ospf_lsa *lsa,
 				return OSPF_GR_FAILURE;
 			break;
 		case RESTARTER_IP_ADDR_TYPE:
+			if (TLV_SIZE(tlvh) <
+			    sizeof(struct grace_tlv_restart_addr)) {
+				zlog_debug("%s: Malformed packet: Invalid addr TLV len:%zu",
+					   __func__, TLV_SIZE(tlvh));
+				return OSPF_GR_FAILURE;
+			}
+
 			restart_addr = (struct grace_tlv_restart_addr *)tlvh;
 			addr->s_addr = restart_addr->addr.s_addr;
 			sum += TLV_SIZE(tlvh);
@@ -524,7 +564,7 @@ void ospf_helper_handle_topo_chg(struct ospf *ospf, struct ospf_lsa *lsa)
 	if (!ospf->active_restarter_cnt)
 		return;
 
-	/* Topo change not required to be hanlded if strict
+	/* Topo change not required to be handled if strict
 	 * LSA check is disbaled for this router.
 	 */
 	if (!ospf->strict_lsa_check)
@@ -929,14 +969,36 @@ static void show_ospf_grace_lsa_info(struct vty *vty, struct ospf_lsa *lsa)
 
 	lsah = (struct lsa_header *)lsa->data;
 
-	length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE;
+	length = ntohs(lsah->length);
+
+	if (length <= OSPF_LSA_HEADER_SIZE) {
+		vty_out(vty, "%% Invalid LSA length: %d\n", length);
+		return;
+	}
+
+	length -= OSPF_LSA_HEADER_SIZE;
 
 	vty_out(vty, "  TLV info:\n");
 
 	for (tlvh = TLV_HDR_TOP(lsah); sum < length;
 	     tlvh = TLV_HDR_NEXT(tlvh)) {
+		/* Check TLV len */
+		if (sum + TLV_SIZE(tlvh) > length) {
+			vty_out(vty, "%% Invalid TLV length: %zu\n",
+				TLV_SIZE(tlvh));
+			return;
+		}
+
 		switch (ntohs(tlvh->type)) {
 		case GRACE_PERIOD_TYPE:
+			if (TLV_SIZE(tlvh) <
+			    sizeof(struct grace_tlv_graceperiod)) {
+				vty_out(vty,
+					"%% Invalid grace TLV length %zu\n",
+					TLV_SIZE(tlvh));
+				return;
+			}
+
 			gracePeriod = (struct grace_tlv_graceperiod *)tlvh;
 			sum += TLV_SIZE(tlvh);
 
@@ -944,6 +1006,14 @@ static void show_ospf_grace_lsa_info(struct vty *vty, struct ospf_lsa *lsa)
 				ntohl(gracePeriod->interval));
 			break;
 		case RESTART_REASON_TYPE:
+			if (TLV_SIZE(tlvh) <
+			    sizeof(struct grace_tlv_restart_reason)) {
+				vty_out(vty,
+					"%% Invalid reason TLV length %zu\n",
+					TLV_SIZE(tlvh));
+				return;
+			}
+
 			grReason = (struct grace_tlv_restart_reason *)tlvh;
 			sum += TLV_SIZE(tlvh);
 
@@ -951,6 +1021,14 @@ static void show_ospf_grace_lsa_info(struct vty *vty, struct ospf_lsa *lsa)
 				ospf_restart_reason_desc[grReason->reason]);
 			break;
 		case RESTARTER_IP_ADDR_TYPE:
+			if (TLV_SIZE(tlvh) <
+			    sizeof(struct grace_tlv_restart_addr)) {
+				vty_out(vty,
+					"%% Invalid addr TLV length %zu\n",
+					TLV_SIZE(tlvh));
+				return;
+			}
+
 			restartAddr = (struct grace_tlv_restart_addr *)tlvh;
 			sum += TLV_SIZE(tlvh);
 
@@ -958,6 +1036,9 @@ static void show_ospf_grace_lsa_info(struct vty *vty, struct ospf_lsa *lsa)
 				inet_ntoa(restartAddr->addr));
 			break;
 		default:
+			vty_out(vty, "   Unknown TLV type %d\n",
+				ntohs(tlvh->type));
+
 			break;
 		}
 	}