lib, pimd, zebra: Provide some insurance against reading bad stream data

This patch does two things:

1) Ensure the decoding of stream data between pim <-> zebra is properly
decoded and we don't read beyond the end of the stream.

2) In zebra when we are freeing memory alloced ensure that we
actually have memory to delete before we do so.

Ticket: CM-27055
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

1 files changed, 62 insertions, 25 deletions

diff --git a/zebra/zebra_mlag.c b/zebra/zebra_mlag.c
index 5b721a8ea..edd71b9f7 100644
--- a/zebra/zebra_mlag.c
+++ b/zebra/zebra_mlag.c
@@ -667,14 +667,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 	int n_len = 0;
 	int rc = 0;
 	char buf[ZLOG_FILTER_LENGTH_MAX];
+	size_t length;
 
 	if (IS_ZEBRA_DEBUG_MLAG)
 		zlog_debug("%s: Entering..", __func__);
 
-	rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg);
+	rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg, &length);
 	if (rc)
 		return rc;
 
+	memset(tmp_buf, 0, ZEBRA_MLAG_BUF_LIMIT);
+
 	if (IS_ZEBRA_DEBUG_MLAG)
 		zlog_debug("%s: Mlag ProtoBuf encoding of message:%s, len:%d",
 			   __func__,
@@ -688,9 +691,10 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 		ZebraMlagMrouteAdd pay_load = ZEBRA_MLAG_MROUTE_ADD__INIT;
 		uint32_t vrf_name_len = 0;
 
-		rc = mlag_lib_decode_mroute_add(s, &msg);
+		rc = mlag_lib_decode_mroute_add(s, &msg, &length);
 		if (rc)
 			return rc;
+
 		vrf_name_len = strlen(msg.vrf_name) + 1;
 		pay_load.vrf_name = XMALLOC(MTYPE_MLAG_PBUF, vrf_name_len);
 		strlcpy(pay_load.vrf_name, msg.vrf_name, vrf_name_len);
@@ -720,7 +724,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 		ZebraMlagMrouteDel pay_load = ZEBRA_MLAG_MROUTE_DEL__INIT;
 		uint32_t vrf_name_len = 0;
 
-		rc = mlag_lib_decode_mroute_del(s, &msg);
+		rc = mlag_lib_decode_mroute_del(s, &msg, &length);
 		if (rc)
 			return rc;
 		vrf_name_len = strlen(msg.vrf_name) + 1;
@@ -749,18 +753,18 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 		ZebraMlagMrouteAddBulk Bulk_msg =
 			ZEBRA_MLAG_MROUTE_ADD_BULK__INIT;
 		ZebraMlagMrouteAdd **pay_load = NULL;
-		int i;
 		bool cleanup = false;
+		uint32_t i, actual;
 
 		Bulk_msg.n_mroute_add = mlag_msg.msg_cnt;
 		pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteAdd *)
 							    * mlag_msg.msg_cnt);
 
-		for (i = 0; i < mlag_msg.msg_cnt; i++) {
+		for (i = 0, actual = 0; i < mlag_msg.msg_cnt; i++, actual++) {
 
 			uint32_t vrf_name_len = 0;
 
-			rc = mlag_lib_decode_mroute_add(s, &msg);
+			rc = mlag_lib_decode_mroute_add(s, &msg, &length);
 			if (rc) {
 				cleanup = true;
 				break;
@@ -796,8 +800,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 							       tmp_buf);
 		}
 
-		for (i = 0; i < mlag_msg.msg_cnt; i++) {
-			XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name);
+		for (i = 0; i < actual; i++) {
+			/*
+			 * The mlag_lib_decode_mroute_add can
+			 * fail to properly decode and cause nothing
+			 * to be allocated.  Prevent a crash
+			 */
+			if (!pay_load[i])
+				continue;
+
+			if (pay_load[i]->vrf_name)
+				XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name);
 			if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE
 			    && pay_load[i]->intf_name)
 				XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name);
@@ -812,18 +825,18 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 		ZebraMlagMrouteDelBulk Bulk_msg =
 			ZEBRA_MLAG_MROUTE_DEL_BULK__INIT;
 		ZebraMlagMrouteDel **pay_load = NULL;
-		int i;
 		bool cleanup = false;
+		uint32_t i, actual;
 
 		Bulk_msg.n_mroute_del = mlag_msg.msg_cnt;
 		pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteDel *)
 							    * mlag_msg.msg_cnt);
 
-		for (i = 0; i < mlag_msg.msg_cnt; i++) {
+		for (i = 0, actual = 0; i < mlag_msg.msg_cnt; i++, actual++) {
 
 			uint32_t vrf_name_len = 0;
 
-			rc = mlag_lib_decode_mroute_del(s, &msg);
+			rc = mlag_lib_decode_mroute_del(s, &msg, &length);
 			if (rc) {
 				cleanup = true;
 				break;
@@ -858,8 +871,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 							       tmp_buf);
 		}
 
-		for (i = 0; i < mlag_msg.msg_cnt; i++) {
-			XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name);
+		for (i = 0; i < actual; i++) {
+			/*
+			 * The mlag_lib_decode_mroute_add can
+			 * fail to properly decode and cause nothing
+			 * to be allocated.  Prevent a crash
+			 */
+			if (!pay_load[i])
+				continue;
+
+			if (pay_load[i]->vrf_name)
+				XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name);
 			if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE
 			    && pay_load[i]->intf_name)
 				XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name);
@@ -915,6 +937,15 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
 	return len;
 }
 
+static void zebra_fill_protobuf_msg(struct stream *s, char *name, int len)
+{
+	int str_len = strlen(name) + 1;
+
+	stream_put(s, name, str_len);
+	/* Fill the rest with Null Character for aligning */
+	stream_put(s, NULL, len - str_len);
+}
+
 int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 				       uint32_t len)
 {
@@ -966,7 +997,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 			/* No Batching */
 			stream_putw(s, MLAG_MSG_NO_BATCH);
 			/* Actual Data */
-			stream_put(s, msg->peerlink, INTERFACE_NAMSIZ);
+			zebra_fill_protobuf_msg(s, msg->peerlink,
+						INTERFACE_NAMSIZ);
 			stream_putl(s, msg->my_role);
 			stream_putl(s, msg->peer_state);
 			zebra_mlag_status_update__free_unpacked(msg, NULL);
@@ -1003,7 +1035,7 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 			/* No Batching */
 			stream_putw(s, MLAG_MSG_NO_BATCH);
 			/* Actual Data */
-			stream_put(s, msg->vrf_name, VRF_NAMSIZ);
+			zebra_fill_protobuf_msg(s, msg->vrf_name, VRF_NAMSIZ);
 
 			stream_putl(s, msg->source_ip);
 			stream_putl(s, msg->group_ip);
@@ -1013,7 +1045,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 			stream_putc(s, msg->am_i_dual_active);
 			stream_putl(s, msg->vrf_id);
 			if (msg->owner_id == MLAG_OWNER_INTERFACE)
-				stream_put(s, msg->intf_name, INTERFACE_NAMSIZ);
+				zebra_fill_protobuf_msg(s, msg->intf_name,
+							INTERFACE_NAMSIZ);
 			else
 				stream_put(s, NULL, INTERFACE_NAMSIZ);
 			zebra_mlag_mroute_add__free_unpacked(msg, NULL);
@@ -1032,15 +1065,15 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 			/* No Batching */
 			stream_putw(s, MLAG_MSG_NO_BATCH);
 			/* Actual Data */
-			stream_put(s, msg->vrf_name, VRF_NAMSIZ);
+			zebra_fill_protobuf_msg(s, msg->vrf_name, VRF_NAMSIZ);
 
 			stream_putl(s, msg->source_ip);
 			stream_putl(s, msg->group_ip);
-			stream_putl(s, msg->group_ip);
 			stream_putl(s, msg->owner_id);
 			stream_putl(s, msg->vrf_id);
 			if (msg->owner_id == MLAG_OWNER_INTERFACE)
-				stream_put(s, msg->intf_name, INTERFACE_NAMSIZ);
+				zebra_fill_protobuf_msg(s, msg->intf_name,
+							INTERFACE_NAMSIZ);
 			else
 				stream_put(s, NULL, INTERFACE_NAMSIZ);
 			zebra_mlag_mroute_del__free_unpacked(msg, NULL);
@@ -1067,7 +1100,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 
 				msg = Bulk_msg->mroute_add[i];
 
-				stream_put(s, msg->vrf_name, VRF_NAMSIZ);
+				zebra_fill_protobuf_msg(s, msg->vrf_name,
+							VRF_NAMSIZ);
 				stream_putl(s, msg->source_ip);
 				stream_putl(s, msg->group_ip);
 				stream_putl(s, msg->cost_to_rp);
@@ -1076,8 +1110,9 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 				stream_putc(s, msg->am_i_dual_active);
 				stream_putl(s, msg->vrf_id);
 				if (msg->owner_id == MLAG_OWNER_INTERFACE)
-					stream_put(s, msg->intf_name,
-						   INTERFACE_NAMSIZ);
+					zebra_fill_protobuf_msg(
+						s, msg->intf_name,
+						INTERFACE_NAMSIZ);
 				else
 					stream_put(s, NULL, INTERFACE_NAMSIZ);
 			}
@@ -1106,14 +1141,16 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
 
 				msg = Bulk_msg->mroute_del[i];
 
-				stream_put(s, msg->vrf_name, VRF_NAMSIZ);
+				zebra_fill_protobuf_msg(s, msg->vrf_name,
+							VRF_NAMSIZ);
 				stream_putl(s, msg->source_ip);
 				stream_putl(s, msg->group_ip);
 				stream_putl(s, msg->owner_id);
 				stream_putl(s, msg->vrf_id);
 				if (msg->owner_id == MLAG_OWNER_INTERFACE)
-					stream_put(s, msg->intf_name,
-						   INTERFACE_NAMSIZ);
+					zebra_fill_protobuf_msg(
+						s, msg->intf_name,
+						INTERFACE_NAMSIZ);
 				else
 					stream_put(s, NULL, INTERFACE_NAMSIZ);
 			}