Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 May 2020 01:13:02 +0000 (UTC)
From:      Michael Tuexen <tuexen@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r360731 - stable/11/sys/netinet
Message-ID:  <202005070113.0471D2F9048359@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: tuexen
Date: Thu May  7 01:13:02 2020
New Revision: 360731
URL: https://svnweb.freebsd.org/changeset/base/360731

Log:
  MFC r351654: Improve handling of cookie parameters in INIT-ACK chunks
  
  Improve the handling of state cookie parameters in INIT-ACK chunks.
  This fixes problem with parameters indicating a zero length or partial
  parameters after an unknown parameter indicating to stop processing. It
  also fixes a problem with state cookie parameters after unknown
  parametes indicating to stop porcessing.
  Thanks to Mark Wodrich from Google for finding two of these issues
  by fuzz testing the userland stack and reporting them in
  https://github.com/sctplab/usrsctp/issues/355
  and
  https://github.com/sctplab/usrsctp/issues/352

Modified:
  stable/11/sys/netinet/sctp_input.c
  stable/11/sys/netinet/sctp_output.c
  stable/11/sys/netinet/sctp_output.h
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netinet/sctp_input.c
==============================================================================
--- stable/11/sys/netinet/sctp_input.c	Thu May  7 01:09:17 2020	(r360730)
+++ stable/11/sys/netinet/sctp_input.c	Thu May  7 01:13:02 2020	(r360731)
@@ -443,22 +443,48 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int 
 {
 	struct sctp_association *asoc;
 	struct mbuf *op_err;
-	int retval, abort_flag;
-	uint32_t initack_limit;
+	int retval, abort_flag, cookie_found;
+	int initack_limit;
 	int nat_friendly = 0;
 
 	/* First verify that we have no illegal param's */
 	abort_flag = 0;
+	cookie_found = 0;
 
 	op_err = sctp_arethere_unrecognized_parameters(m,
 	    (offset + sizeof(struct sctp_init_chunk)),
-	    &abort_flag, (struct sctp_chunkhdr *)cp, &nat_friendly);
+	    &abort_flag, (struct sctp_chunkhdr *)cp,
+	    &nat_friendly, &cookie_found);
 	if (abort_flag) {
 		/* Send an abort and notify peer */
 		sctp_abort_an_association(stcb->sctp_ep, stcb, op_err, SCTP_SO_NOT_LOCKED);
 		*abort_no_unlock = 1;
 		return (-1);
 	}
+	if (!cookie_found) {
+		uint16_t len;
+
+		len = (uint16_t)(sizeof(struct sctp_error_missing_param) + sizeof(uint16_t));
+		/* We abort with an error of missing mandatory param */
+		op_err = sctp_get_mbuf_for_msg(len, 0, M_NOWAIT, 1, MT_DATA);
+		if (op_err != NULL) {
+			struct sctp_error_missing_param *cause;
+
+			SCTP_BUF_LEN(op_err) = len;
+			cause = mtod(op_err, struct sctp_error_missing_param *);
+			/* Subtract the reserved param */
+			cause->cause.code = htons(SCTP_CAUSE_MISSING_PARAM);
+			cause->cause.length = htons(len);
+			cause->num_missing_params = htonl(1);
+			cause->type[0] = htons(SCTP_STATE_COOKIE);
+		}
+		sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen,
+		    src, dst, sh, op_err,
+		    mflowtype, mflowid,
+		    vrf_id, net->port);
+		*abort_no_unlock = 1;
+		return (-3);
+	}
 	asoc = &stcb->asoc;
 	asoc->peer_supports_nat = (uint8_t)nat_friendly;
 	/* process the peer's parameters in the INIT-ACK */
@@ -523,40 +549,8 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int 
 	/* calculate the RTO */
 	net->RTO = sctp_calculate_rto(stcb, asoc, net, &asoc->time_entered,
 	    SCTP_RTT_FROM_NON_DATA);
-	retval = sctp_send_cookie_echo(m, offset, stcb, net);
-	if (retval < 0) {
-		/*
-		 * No cookie, we probably should send a op error. But in any
-		 * case if there is no cookie in the INIT-ACK, we can
-		 * abandon the peer, its broke.
-		 */
-		if (retval == -3) {
-			uint16_t len;
-
-			len = (uint16_t)(sizeof(struct sctp_error_missing_param) + sizeof(uint16_t));
-			/* We abort with an error of missing mandatory param */
-			op_err = sctp_get_mbuf_for_msg(len, 0, M_NOWAIT, 1, MT_DATA);
-			if (op_err != NULL) {
-				struct sctp_error_missing_param *cause;
-
-				SCTP_BUF_LEN(op_err) = len;
-				cause = mtod(op_err, struct sctp_error_missing_param *);
-				/* Subtract the reserved param */
-				cause->cause.code = htons(SCTP_CAUSE_MISSING_PARAM);
-				cause->cause.length = htons(len);
-				cause->num_missing_params = htonl(1);
-				cause->type[0] = htons(SCTP_STATE_COOKIE);
-			}
-			sctp_abort_association(stcb->sctp_ep, stcb, m, iphlen,
-			    src, dst, sh, op_err,
-			    mflowtype, mflowid,
-			    vrf_id, net->port);
-			*abort_no_unlock = 1;
-		}
-		return (retval);
-	}
-
-	return (0);
+	retval = sctp_send_cookie_echo(m, offset, initack_limit, stcb, net);
+	return (retval);
 }
 
 static void

Modified: stable/11/sys/netinet/sctp_output.c
==============================================================================
--- stable/11/sys/netinet/sctp_output.c	Thu May  7 01:09:17 2020	(r360730)
+++ stable/11/sys/netinet/sctp_output.c	Thu May  7 01:13:02 2020	(r360731)
@@ -4963,7 +4963,10 @@ sctp_send_initiate(struct sctp_inpcb *inp, struct sctp
 
 struct mbuf *
 sctp_arethere_unrecognized_parameters(struct mbuf *in_initpkt,
-    int param_offset, int *abort_processing, struct sctp_chunkhdr *cp, int *nat_friendly)
+    int param_offset, int *abort_processing,
+    struct sctp_chunkhdr *cp,
+    int *nat_friendly,
+    int *cookie_found)
 {
 	/*
 	 * Given a mbuf containing an INIT or INIT-ACK with the param_offset
@@ -4986,6 +4989,9 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_
 	uint16_t ptype, plen, padded_size;
 
 	*abort_processing = 0;
+	if (cookie_found != NULL) {
+		*cookie_found = 0;
+	}
 	mat = in_initpkt;
 	limit = ntohs(cp->chunk_length) - sizeof(struct sctp_init_chunk);
 	at = param_offset;
@@ -5014,12 +5020,17 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_
 		switch (ptype) {
 			/* Param's with variable size */
 		case SCTP_HEARTBEAT_INFO:
-		case SCTP_STATE_COOKIE:
 		case SCTP_UNRECOG_PARAM:
 		case SCTP_ERROR_CAUSE_IND:
 			/* ok skip fwd */
 			at += padded_size;
 			break;
+		case SCTP_STATE_COOKIE:
+			if (cookie_found != NULL) {
+				*cookie_found = 1;
+			}
+			at += padded_size;
+			break;
 			/* Param's with variable size within a range */
 		case SCTP_CHUNK_LIST:
 		case SCTP_SUPPORTED_CHUNK_EXT:
@@ -5561,7 +5572,9 @@ sctp_send_initiate_ack(struct sctp_inpcb *inp, struct 
 	abort_flag = 0;
 	op_err = sctp_arethere_unrecognized_parameters(init_pkt,
 	    (offset + sizeof(struct sctp_init_chunk)),
-	    &abort_flag, (struct sctp_chunkhdr *)init_chk, &nat_friendly);
+	    &abort_flag,
+	    (struct sctp_chunkhdr *)init_chk,
+	    &nat_friendly, NULL);
 	if (abort_flag) {
 do_a_abort:
 		if (op_err == NULL) {
@@ -8995,7 +9008,7 @@ sctp_queue_op_err(struct sctp_tcb *stcb, struct mbuf *
 
 int
 sctp_send_cookie_echo(struct mbuf *m,
-    int offset,
+    int offset, int limit,
     struct sctp_tcb *stcb,
     struct sctp_nets *net)
 {
@@ -9021,18 +9034,30 @@ sctp_send_cookie_echo(struct mbuf *m,
 		}
 		ptype = ntohs(phdr->param_type);
 		plen = ntohs(phdr->param_length);
+		if (plen < sizeof(struct sctp_paramhdr)) {
+			return (-6);
+		}
 		if (ptype == SCTP_STATE_COOKIE) {
 			int pad;
 
 			/* found the cookie */
-			if ((pad = (plen % 4))) {
-				plen += 4 - pad;
+			if (at + plen > limit) {
+				return (-7);
 			}
 			cookie = SCTP_M_COPYM(m, at, plen, M_NOWAIT);
 			if (cookie == NULL) {
 				/* No memory */
 				return (-2);
 			}
+			if ((pad = (plen % 4)) > 0) {
+				pad = 4 - pad;
+			}
+			if (pad > 0) {
+				cookie = sctp_pad_lastmbuf(cookie, pad, NULL);
+				if (cookie == NULL) {
+					return (-8);
+				}
+			}
 #ifdef SCTP_MBUF_LOGGING
 			if (SCTP_BASE_SYSCTL(sctp_logging_level) & SCTP_MBUF_LOGGING_ENABLE) {
 				sctp_log_mbc(cookie, SCTP_MBUF_ICOPY);
@@ -9058,7 +9083,7 @@ sctp_send_cookie_echo(struct mbuf *m,
 	chk->rec.chunk_id.id = SCTP_COOKIE_ECHO;
 	chk->rec.chunk_id.can_take_data = 0;
 	chk->flags = CHUNK_FLAGS_FRAGMENT_OK;
-	chk->send_size = plen;
+	chk->send_size = SCTP_SIZE32(plen);
 	chk->sent = SCTP_DATAGRAM_UNSENT;
 	chk->snd_count = 0;
 	chk->asoc = &stcb->asoc;

Modified: stable/11/sys/netinet/sctp_output.h
==============================================================================
--- stable/11/sys/netinet/sctp_output.h	Thu May  7 01:09:17 2020	(r360730)
+++ stable/11/sys/netinet/sctp_output.h	Thu May  7 01:13:02 2020	(r360731)
@@ -90,11 +90,11 @@ sctp_send_initiate_ack(struct sctp_inpcb *, struct sct
 
 struct mbuf *
 sctp_arethere_unrecognized_parameters(struct mbuf *, int, int *,
-    struct sctp_chunkhdr *, int *);
+    struct sctp_chunkhdr *, int *, int *);
 void sctp_queue_op_err(struct sctp_tcb *, struct mbuf *);
 
 int
-sctp_send_cookie_echo(struct mbuf *, int, struct sctp_tcb *,
+sctp_send_cookie_echo(struct mbuf *, int, int, struct sctp_tcb *,
     struct sctp_nets *);
 
 void sctp_send_cookie_ack(struct sctp_tcb *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005070113.0471D2F9048359>