[Iscsitarget-devel] [patch] fix iSNS bounds checking

[Ross S. W. W. <RW...@me...>]

This patch fixes the iSNS code's bounds checking on SCN packets.

Fixes a couple buffer overflow scenarios per CVE-2010-2221.

Also fixes a memory leak with initiator ACLs.


Signed-off-by: Ross Walker

Index: usr/isns.c
===================================================================
--- usr/isns.c	(revision 339)
+++ usr/isns.c	(working copy)
@@ -447,6 +447,7 @@ static void free_all_acl(struct target *
 	while (!list_empty(&target->isns_head)) {
 		ini = list_entry(target->isns_head.q_forw, typeof(*ini), ilist);
 		remque(&ini->ilist);
+		free(ini);
 	}
 }
 
@@ -600,18 +601,36 @@ static char *print_scn_pdu(struct isns_h
 	while (length) {
 		uint32_t vlen = ntohl(tlv->length);
 
+		if (vlen + sizeof(*tlv) > length)
+			vlen = length - sizeof(*tlv);
+
 		switch (ntohl(tlv->tag)) {
 		case ISNS_ATTR_ISCSI_NAME:
-			log_error("scn name: %u, %s", vlen, (char *) tlv->value);
-			if (!name)
-				name = (char *) tlv->value;
+			if (vlen) {
+				size_t slen = vlen - 1;
+
+				if (slen > ISCSI_NAME_LEN)
+					slen = ISCSI_NAME_LEN;
+
+				*((char *) tlv->value + slen) = 0;
+
+				log_error("scn name: %u, %s", vlen,
+					  (char *) tlv->value);
+
+				if (!name)
+					name = (char *) tlv->value;
+			}
 			break;
 		case ISNS_ATTR_TIMESTAMP:
-/* 			log_error("%u : %u : %" PRIx64, ntohl(tlv->tag), vlen, */
-/* 				  *((uint64_t *) tlv->value)); */
+/*			if (vlen == 8)
+				log_error("%u : %u : %" PRIx64, ntohl(tlv->tag),
+					  vlen, *((uint64_t *) tlv->value));
+*/
 			break;
 		case ISNS_ATTR_ISCSI_SCN_BITMAP:
-			log_error("scn bitmap : %x", *((uint32_t *) tlv->value));
+			if (vlen == 4)
+				log_error("scn bitmap : %x",
+					  *((uint32_t *) tlv->value));
 			break;
 		}
 
@@ -671,24 +690,45 @@ found:
 
 	/* skip status */
 	tlv = (struct isns_tlv *) ((char *) hdr->pdu + 4);
+
+	if (length < 4)
+		goto free_qry_mgmt;
+
 	length -= 4;
 
 	while (length) {
 		uint32_t vlen = ntohl(tlv->length);
 
+		if (vlen + sizeof(*tlv) > length)
+			vlen = length - sizeof(*tlv);
+
 		switch (ntohl(tlv->tag)) {
 		case ISNS_ATTR_ISCSI_NAME:
-			name = (char *) tlv->value;
+			if (vlen) {
+				size_t slen = vlen - 1;
+
+				if (slen > ISCSI_NAME_LEN)
+					slen = ISCSI_NAME_LEN;
+
+				*((char *) tlv->value + slen) = 0;
+
+				name = (char *) tlv->value;
+			} else
+				name = NULL;
 			break;
 		case ISNS_ATTR_ISCSI_NODE_TYPE:
-			if (ntohl(*(tlv->value)) == ISNS_NODE_INITIATOR && name) {
+			if (vlen == 4 && name &&
+			    ntohl(*(tlv->value)) == ISNS_NODE_INITIATOR) {
 				log_error("%s %d: %s", __FUNCTION__, __LINE__,
 					  (char *) name);
+
 				ini = malloc(sizeof(*ini));
 				if (!ini)
 					goto free_qry_mgmt;
+
 				snprintf(ini->name, sizeof(ini->name), "%s",
 					 name);
+
 				insque(&ini->ilist, &target->isns_head);
 			} else
 				name = NULL;

______________________________________________________________________
This e-mail, and any attachments thereto, is intended only for use by
the addressee(s) named herein and may contain legally privileged
and/or confidential information. If you are not the intended recipient
of this e-mail, you are hereby notified that any dissemination,
distribution or copying of this e-mail, and any attachments thereto,
is strictly prohibited. If you have received this e-mail in error,
please immediately notify the sender and permanently delete the
original and any copy or printout thereof.