From owner-svn-src-releng@freebsd.org Thu Oct 10 18:19:24 2019 Return-Path: Delivered-To: svn-src-releng@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 16B62145A09; Thu, 10 Oct 2019 18:19:24 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46pzrC6jznz4PVT; Thu, 10 Oct 2019 18:19:23 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id C5B9A5DBF; Thu, 10 Oct 2019 18:19:23 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x9AIJNTm089207; Thu, 10 Oct 2019 18:19:23 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x9AIJMKj089203; Thu, 10 Oct 2019 18:19:23 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <201910101819.x9AIJMKj089203@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Thu, 10 Oct 2019 18:19:22 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org Subject: svn commit: r353410 - releng/12.1/sys/netinet X-SVN-Group: releng X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: releng/12.1/sys/netinet X-SVN-Commit-Revision: 353410 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-releng@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the release engineering / security commits to the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Oct 2019 18:19:24 -0000 Author: tuexen Date: Thu Oct 10 18:19:22 2019 New Revision: 353410 URL: https://svnweb.freebsd.org/changeset/base/353410 Log: MFS r353395: Add missing input validation. This could result in reading from uninitialized memory. The issue was found by OSS-Fuzz for usrsctp and reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17780 MFS r353396: Cleanup sctp_asconf_error_response() and ensure that the parameter is padded as required. This fixes the followig bug reported by OSS-Fuzz for the usersctp stack: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17790 MFS r353397: When skipping the address parameter, take the padding into account. MFS r353398: Fix the adding of padding to COOKIE-ECHO chunks. Thanks to Mark Wodrich who found this issue while fuzz testing the usrsctp stack and reported the issue in https://github.com/sctplab/usrsctp/issues/382 MFS r353399: Plumb an mbuf leak found by Mark Wodrich from Google by fuzz testing the userland stack and reporting it in: https://github.com/sctplab/usrsctp/issues/396 MFS r353400: Fix a use after free bug when removing remote addresses. This bug was found by OSS-Fuzz and reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=18004 MFS r353401: Plumb an mbuf leak in a code path that should not be taken. Also avoid that this path is taken by setting the tail pointer correctly. There is still bug related to handling unordered unfragmented messages which were delayed in deferred handling. This issue was found by OSS-Fuzz testing the usrsctp stack and reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17794 MFS r353403: Validate length before use it, not vice versa. r353060 should have contained this... This fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=18070 Approved by: re (gjb@) Modified: releng/12.1/sys/netinet/sctp_asconf.c releng/12.1/sys/netinet/sctp_indata.c releng/12.1/sys/netinet/sctp_input.c releng/12.1/sys/netinet/sctp_output.c Directory Properties: releng/12.1/ (props changed) Modified: releng/12.1/sys/netinet/sctp_asconf.c ============================================================================== --- releng/12.1/sys/netinet/sctp_asconf.c Thu Oct 10 17:29:47 2019 (r353409) +++ releng/12.1/sys/netinet/sctp_asconf.c Thu Oct 10 18:19:22 2019 (r353410) @@ -105,42 +105,47 @@ sctp_asconf_error_response(uint32_t id, uint16_t cause struct mbuf *m_reply = NULL; struct sctp_asconf_paramhdr *aph; struct sctp_error_cause *error; + size_t buf_len; + uint16_t i, param_length, cause_length, padding_length; uint8_t *tlv; - m_reply = sctp_get_mbuf_for_msg((sizeof(struct sctp_asconf_paramhdr) + - tlv_length + - sizeof(struct sctp_error_cause)), - 0, M_NOWAIT, 1, MT_DATA); + if (error_tlv == NULL) { + tlv_length = 0; + } + cause_length = sizeof(struct sctp_error_cause) + tlv_length; + param_length = sizeof(struct sctp_asconf_paramhdr) + cause_length; + padding_length = tlv_length % 4; + if (padding_length != 0) { + padding_length = 4 - padding_length; + } + buf_len = param_length + padding_length; + if (buf_len > MLEN) { + SCTPDBG(SCTP_DEBUG_ASCONF1, + "asconf_error_response: tlv_length (%xh) too big\n", + tlv_length); + return (NULL); + } + m_reply = sctp_get_mbuf_for_msg(buf_len, 0, M_NOWAIT, 1, MT_DATA); if (m_reply == NULL) { SCTPDBG(SCTP_DEBUG_ASCONF1, "asconf_error_response: couldn't get mbuf!\n"); return (NULL); } aph = mtod(m_reply, struct sctp_asconf_paramhdr *); - error = (struct sctp_error_cause *)(aph + 1); - - aph->correlation_id = id; aph->ph.param_type = htons(SCTP_ERROR_CAUSE_IND); + aph->ph.param_length = htons(param_length); + aph->correlation_id = id; + error = (struct sctp_error_cause *)(aph + 1); error->code = htons(cause); - error->length = tlv_length + sizeof(struct sctp_error_cause); - aph->ph.param_length = error->length + - sizeof(struct sctp_asconf_paramhdr); - - if (aph->ph.param_length > MLEN) { - SCTPDBG(SCTP_DEBUG_ASCONF1, - "asconf_error_response: tlv_length (%xh) too big\n", - tlv_length); - sctp_m_freem(m_reply); /* discard */ - return (NULL); - } + error->length = htons(cause_length); if (error_tlv != NULL) { tlv = (uint8_t *)(error + 1); memcpy(tlv, error_tlv, tlv_length); + for (i = 0; i < padding_length; i++) { + tlv[tlv_length + i] = 0; + } } - SCTP_BUF_LEN(m_reply) = aph->ph.param_length; - error->length = htons(error->length); - aph->ph.param_length = htons(aph->ph.param_length); - + SCTP_BUF_LEN(m_reply) = buf_len; return (m_reply); } @@ -169,10 +174,16 @@ sctp_process_asconf_add_ip(struct sockaddr *src, struc #endif aparam_length = ntohs(aph->ph.param_length); + if (aparam_length < sizeof(struct sctp_asconf_paramhdr) + sizeof(struct sctp_paramhdr)) { + return (NULL); + } ph = (struct sctp_paramhdr *)(aph + 1); param_type = ntohs(ph->param_type); #if defined(INET) || defined(INET6) param_length = ntohs(ph->param_length); + if (param_length + sizeof(struct sctp_asconf_paramhdr) != aparam_length) { + return (NULL); + } #endif sa = &store.sa; switch (param_type) { @@ -272,7 +283,7 @@ sctp_process_asconf_add_ip(struct sockaddr *src, struc static int sctp_asconf_del_remote_addrs_except(struct sctp_tcb *stcb, struct sockaddr *src) { - struct sctp_nets *src_net, *net; + struct sctp_nets *src_net, *net, *nnet; /* make sure the source address exists as a destination net */ src_net = sctp_findnet(stcb, src); @@ -282,10 +293,9 @@ sctp_asconf_del_remote_addrs_except(struct sctp_tcb *s } /* delete all destination addresses except the source */ - TAILQ_FOREACH(net, &stcb->asoc.nets, sctp_next) { + TAILQ_FOREACH_SAFE(net, &stcb->asoc.nets, sctp_next, nnet) { if (net != src_net) { /* delete this address */ - sctp_remove_net(stcb, net); SCTPDBG(SCTP_DEBUG_ASCONF1, "asconf_del_remote_addrs_except: deleting "); SCTPDBG_ADDR(SCTP_DEBUG_ASCONF1, @@ -293,6 +303,7 @@ sctp_asconf_del_remote_addrs_except(struct sctp_tcb *s /* notify upper layer */ sctp_ulp_notify(SCTP_NOTIFY_ASCONF_DELETE_IP, stcb, 0, (struct sockaddr *)&net->ro._l_addr, SCTP_SO_NOT_LOCKED); + sctp_remove_net(stcb, net); } } return (0); @@ -323,10 +334,16 @@ sctp_process_asconf_delete_ip(struct sockaddr *src, #endif aparam_length = ntohs(aph->ph.param_length); + if (aparam_length < sizeof(struct sctp_asconf_paramhdr) + sizeof(struct sctp_paramhdr)) { + return (NULL); + } ph = (struct sctp_paramhdr *)(aph + 1); param_type = ntohs(ph->param_type); #if defined(INET) || defined(INET6) param_length = ntohs(ph->param_length); + if (param_length + sizeof(struct sctp_asconf_paramhdr) != aparam_length) { + return (NULL); + } #endif sa = &store.sa; switch (param_type) { @@ -454,10 +471,16 @@ sctp_process_asconf_set_primary(struct sockaddr *src, #endif aparam_length = ntohs(aph->ph.param_length); + if (aparam_length < sizeof(struct sctp_asconf_paramhdr) + sizeof(struct sctp_paramhdr)) { + return (NULL); + } ph = (struct sctp_paramhdr *)(aph + 1); param_type = ntohs(ph->param_type); #if defined(INET) || defined(INET6) param_length = ntohs(ph->param_length); + if (param_length + sizeof(struct sctp_asconf_paramhdr) != aparam_length) { + return (NULL); + } #endif sa = &store.sa; switch (param_type) { @@ -676,8 +699,8 @@ sctp_handle_asconf(struct mbuf *m, unsigned int offset sctp_m_freem(m_ack); return; } - /* param_length is already validated in process_control... */ - offset += ntohs(p_addr->ph.param_length); /* skip lookup addr */ + /* skip lookup addr */ + offset += SCTP_SIZE32(ntohs(p_addr->ph.param_length)); /* get pointer to first asconf param in ASCONF */ aph = (struct sctp_asconf_paramhdr *)sctp_m_getptr(m, offset, sizeof(struct sctp_asconf_paramhdr), (uint8_t *)&aparam_buf); if (aph == NULL) { @@ -762,8 +785,6 @@ sctp_handle_asconf(struct mbuf *m, unsigned int offset if (m_result != NULL) { SCTP_BUF_NEXT(m_tail) = m_result; m_tail = m_result; - /* update lengths, make sure it's aligned too */ - SCTP_BUF_LEN(m_result) = SCTP_SIZE32(SCTP_BUF_LEN(m_result)); ack_cp->ch.chunk_length += SCTP_BUF_LEN(m_result); /* set flag to force success reports */ error = 1; Modified: releng/12.1/sys/netinet/sctp_indata.c ============================================================================== --- releng/12.1/sys/netinet/sctp_indata.c Thu Oct 10 17:29:47 2019 (r353409) +++ releng/12.1/sys/netinet/sctp_indata.c Thu Oct 10 18:19:22 2019 (r353410) @@ -716,6 +716,7 @@ sctp_add_to_tail_pointer(struct sctp_queued_to_read *c } if (control->tail_mbuf == NULL) { /* TSNH */ + sctp_m_freem(control->data); control->data = m; sctp_setup_tail_pointer(control); return; @@ -2119,10 +2120,13 @@ sctp_process_a_data_chunk(struct sctp_tcb *stcb, struc struct mbuf *mm; control->data = dmbuf; + control->tail_mbuf = NULL; for (mm = control->data; mm; mm = mm->m_next) { control->length += SCTP_BUF_LEN(mm); + if (SCTP_BUF_NEXT(mm) == NULL) { + control->tail_mbuf = mm; + } } - control->tail_mbuf = NULL; control->end_added = 1; control->last_frag_seen = 1; control->first_frag_seen = 1; Modified: releng/12.1/sys/netinet/sctp_input.c ============================================================================== --- releng/12.1/sys/netinet/sctp_input.c Thu Oct 10 17:29:47 2019 (r353409) +++ releng/12.1/sys/netinet/sctp_input.c Thu Oct 10 18:19:22 2019 (r353410) @@ -465,6 +465,10 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int if (!cookie_found) { uint16_t len; + /* Only report the missing cookie parameter */ + if (op_err != NULL) { + sctp_m_freem(op_err); + } 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); Modified: releng/12.1/sys/netinet/sctp_output.c ============================================================================== --- releng/12.1/sys/netinet/sctp_output.c Thu Oct 10 17:29:47 2019 (r353409) +++ releng/12.1/sys/netinet/sctp_output.c Thu Oct 10 18:19:22 2019 (r353410) @@ -9059,8 +9059,7 @@ sctp_send_cookie_echo(struct mbuf *m, pad = 4 - pad; } if (pad > 0) { - cookie = sctp_pad_lastmbuf(cookie, pad, NULL); - if (cookie == NULL) { + if (sctp_pad_lastmbuf(cookie, pad, NULL) == NULL) { return (-8); } }