From owner-svn-src-head@freebsd.org Fri Jun 23 21:01:59 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 87D7BD89E84; Fri, 23 Jun 2017 21:01:59 +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 mx1.freebsd.org (Postfix) with ESMTPS id 5038B741D7; Fri, 23 Jun 2017 21:01:59 +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 v5NL1wFL024607; Fri, 23 Jun 2017 21:01:58 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v5NL1vKo024603; Fri, 23 Jun 2017 21:01:57 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <201706232101.v5NL1vKo024603@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Fri, 23 Jun 2017 21:01:57 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r320300 - head/sys/netinet X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Jun 2017 21:01:59 -0000 Author: tuexen Date: Fri Jun 23 21:01:57 2017 New Revision: 320300 URL: https://svnweb.freebsd.org/changeset/base/320300 Log: Handle sctp_get_next_param() in a consistent way. This addresses an issue found by Felix Weinrank using libfuzz. While there, use also consistent nameing. MFC after: 3 days Modified: head/sys/netinet/sctp_auth.c head/sys/netinet/sctp_input.c head/sys/netinet/sctp_output.c head/sys/netinet/sctp_pcb.c Modified: head/sys/netinet/sctp_auth.c ============================================================================== --- head/sys/netinet/sctp_auth.c Fri Jun 23 20:49:23 2017 (r320299) +++ head/sys/netinet/sctp_auth.c Fri Jun 23 21:01:57 2017 (r320300) @@ -1434,7 +1434,7 @@ sctp_auth_get_cookie_params(struct sctp_tcb *stcb, str if (plen > sizeof(random_store)) break; phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)random_store, min(plen, sizeof(random_store))); + (struct sctp_paramhdr *)random_store, plen); if (phdr == NULL) return; /* save the random and length for the key */ @@ -1447,7 +1447,7 @@ sctp_auth_get_cookie_params(struct sctp_tcb *stcb, str if (plen > sizeof(hmacs_store)) break; phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)hmacs_store, min(plen, sizeof(hmacs_store))); + (struct sctp_paramhdr *)hmacs_store, plen); if (phdr == NULL) return; /* save the hmacs list and num for the key */ @@ -1469,7 +1469,7 @@ sctp_auth_get_cookie_params(struct sctp_tcb *stcb, str if (plen > sizeof(chunks_store)) break; phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)chunks_store, min(plen, sizeof(chunks_store))); + (struct sctp_paramhdr *)chunks_store, plen); if (phdr == NULL) return; chunks = (struct sctp_auth_chunk_list *)phdr; @@ -1814,7 +1814,7 @@ sctp_notify_authentication(struct sctp_tcb *stcb, uint int sctp_validate_init_auth_params(struct mbuf *m, int offset, int limit) { - struct sctp_paramhdr *phdr, parm_buf; + struct sctp_paramhdr *phdr, param_buf; uint16_t ptype, plen; int peer_supports_asconf = 0; int peer_supports_auth = 0; @@ -1823,7 +1823,7 @@ sctp_validate_init_auth_params(struct mbuf *m, int off uint8_t saw_asconf_ack = 0; /* go through each of the params. */ - phdr = sctp_get_next_param(m, offset, &parm_buf, sizeof(parm_buf)); + phdr = sctp_get_next_param(m, offset, ¶m_buf, sizeof(param_buf)); while (phdr) { ptype = ntohs(phdr->param_type); plen = ntohs(phdr->param_length); @@ -1837,11 +1837,15 @@ sctp_validate_init_auth_params(struct mbuf *m, int off if (ptype == SCTP_SUPPORTED_CHUNK_EXT) { /* A supported extension chunk */ struct sctp_supported_chunk_types_param *pr_supported; - uint8_t local_store[SCTP_PARAM_BUFFER_SIZE]; + uint8_t local_store[SCTP_SMALL_CHUNK_STORE]; int num_ent, i; + if (plen > sizeof(local_store)) { + break; + } phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)&local_store, min(plen, sizeof(local_store))); + (struct sctp_paramhdr *)&local_store, + plen); if (phdr == NULL) { return (-1); } @@ -1859,7 +1863,6 @@ sctp_validate_init_auth_params(struct mbuf *m, int off } } } else if (ptype == SCTP_RANDOM) { - got_random = 1; /* enforce the random length */ if (plen != (sizeof(struct sctp_auth_random) + SCTP_AUTH_RANDOM_SIZE_REQUIRED)) { @@ -1867,20 +1870,23 @@ sctp_validate_init_auth_params(struct mbuf *m, int off "SCTP: invalid RANDOM len\n"); return (-1); } + got_random = 1; } else if (ptype == SCTP_HMAC_LIST) { - uint8_t store[SCTP_PARAM_BUFFER_SIZE]; struct sctp_auth_hmac_algo *hmacs; + uint8_t store[SCTP_PARAM_BUFFER_SIZE]; int num_hmacs; - if (plen > sizeof(store)) + if (plen > sizeof(store)) { break; + } phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)store, min(plen, sizeof(store))); - if (phdr == NULL) + (struct sctp_paramhdr *)store, + plen); + if (phdr == NULL) { return (-1); + } hmacs = (struct sctp_auth_hmac_algo *)phdr; - num_hmacs = (plen - sizeof(*hmacs)) / - sizeof(hmacs->hmac_ids[0]); + num_hmacs = (plen - sizeof(*hmacs)) / sizeof(hmacs->hmac_ids[0]); /* validate the hmac list */ if (sctp_verify_hmac_param(hmacs, num_hmacs)) { SCTPDBG(SCTP_DEBUG_AUTH1, @@ -1889,18 +1895,19 @@ sctp_validate_init_auth_params(struct mbuf *m, int off } got_hmacs = 1; } else if (ptype == SCTP_CHUNK_LIST) { - int i, num_chunks; + struct sctp_auth_chunk_list *chunks; uint8_t chunks_store[SCTP_SMALL_CHUNK_STORE]; + int i, num_chunks; - /* did the peer send a non-empty chunk list? */ - struct sctp_auth_chunk_list *chunks = NULL; - + if (plen > sizeof(chunks_store)) { + break; + } phdr = sctp_get_next_param(m, offset, (struct sctp_paramhdr *)chunks_store, - min(plen, sizeof(chunks_store))); - if (phdr == NULL) + plen); + if (phdr == NULL) { return (-1); - + } /*- * Flip through the list and mark that the * peer supports asconf/asconf_ack. @@ -1922,8 +1929,8 @@ sctp_validate_init_auth_params(struct mbuf *m, int off if (offset >= limit) { break; } - phdr = sctp_get_next_param(m, offset, &parm_buf, - sizeof(parm_buf)); + phdr = sctp_get_next_param(m, offset, ¶m_buf, + sizeof(param_buf)); } /* validate authentication required parameters */ if (got_random && got_hmacs) { Modified: head/sys/netinet/sctp_input.c ============================================================================== --- head/sys/netinet/sctp_input.c Fri Jun 23 20:49:23 2017 (r320299) +++ head/sys/netinet/sctp_input.c Fri Jun 23 21:01:57 2017 (r320300) @@ -3617,7 +3617,7 @@ sctp_handle_stream_reset_response(struct sctp_tcb *stc struct sctp_stream_reset_response *respin) { uint16_t type; - int lparm_len; + int lparam_len; struct sctp_association *asoc = &stcb->asoc; struct sctp_tmit_chunk *chk; struct sctp_stream_reset_request *req_param; @@ -3634,12 +3634,12 @@ sctp_handle_stream_reset_response(struct sctp_tcb *stc if (req_param != NULL) { stcb->asoc.str_reset_seq_out++; type = ntohs(req_param->ph.param_type); - lparm_len = ntohs(req_param->ph.param_length); + lparam_len = ntohs(req_param->ph.param_length); if (type == SCTP_STR_RESET_OUT_REQUEST) { int no_clear = 0; req_out_param = (struct sctp_stream_reset_out_request *)req_param; - number_entries = (lparm_len - sizeof(struct sctp_stream_reset_out_request)) / sizeof(uint16_t); + number_entries = (lparam_len - sizeof(struct sctp_stream_reset_out_request)) / sizeof(uint16_t); asoc->stream_reset_out_is_outstanding = 0; if (asoc->stream_reset_outstanding) asoc->stream_reset_outstanding--; @@ -3665,7 +3665,7 @@ sctp_handle_stream_reset_response(struct sctp_tcb *stc } } else if (type == SCTP_STR_RESET_IN_REQUEST) { req_in_param = (struct sctp_stream_reset_in_request *)req_param; - number_entries = (lparm_len - sizeof(struct sctp_stream_reset_in_request)) / sizeof(uint16_t); + number_entries = (lparam_len - sizeof(struct sctp_stream_reset_in_request)) / sizeof(uint16_t); if (asoc->stream_reset_outstanding) asoc->stream_reset_outstanding--; if (action == SCTP_STREAM_RESET_RESULT_DENIED) { Modified: head/sys/netinet/sctp_output.c ============================================================================== --- head/sys/netinet/sctp_output.c Fri Jun 23 20:49:23 2017 (r320299) +++ head/sys/netinet/sctp_output.c Fri Jun 23 21:01:57 2017 (r320300) @@ -1940,7 +1940,7 @@ static struct mbuf * sctp_add_addr_to_mbuf(struct mbuf *m, struct sctp_ifa *ifa, uint16_t *len) { #if defined(INET) || defined(INET6) - struct sctp_paramhdr *parmh; + struct sctp_paramhdr *paramh; struct mbuf *mret; uint16_t plen; #endif @@ -1962,7 +1962,7 @@ sctp_add_addr_to_mbuf(struct mbuf *m, struct sctp_ifa #if defined(INET) || defined(INET6) if (M_TRAILINGSPACE(m) >= plen) { /* easy side we just drop it on the end */ - parmh = (struct sctp_paramhdr *)(SCTP_BUF_AT(m, SCTP_BUF_LEN(m))); + paramh = (struct sctp_paramhdr *)(SCTP_BUF_AT(m, SCTP_BUF_LEN(m))); mret = m; } else { /* Need more space */ @@ -1976,7 +1976,7 @@ sctp_add_addr_to_mbuf(struct mbuf *m, struct sctp_ifa return (m); } mret = SCTP_BUF_NEXT(mret); - parmh = mtod(mret, struct sctp_paramhdr *); + paramh = mtod(mret, struct sctp_paramhdr *); } /* now add the parameter */ switch (ifa->address.sa.sa_family) { @@ -1987,9 +1987,9 @@ sctp_add_addr_to_mbuf(struct mbuf *m, struct sctp_ifa struct sockaddr_in *sin; sin = &ifa->address.sin; - ipv4p = (struct sctp_ipv4addr_param *)parmh; - parmh->param_type = htons(SCTP_IPV4_ADDRESS); - parmh->param_length = htons(plen); + ipv4p = (struct sctp_ipv4addr_param *)paramh; + paramh->param_type = htons(SCTP_IPV4_ADDRESS); + paramh->param_length = htons(plen); ipv4p->addr = sin->sin_addr.s_addr; SCTP_BUF_LEN(mret) += plen; break; @@ -2002,9 +2002,9 @@ sctp_add_addr_to_mbuf(struct mbuf *m, struct sctp_ifa struct sockaddr_in6 *sin6; sin6 = &ifa->address.sin6; - ipv6p = (struct sctp_ipv6addr_param *)parmh; - parmh->param_type = htons(SCTP_IPV6_ADDRESS); - parmh->param_length = htons(plen); + ipv6p = (struct sctp_ipv6addr_param *)paramh; + paramh->param_type = htons(SCTP_IPV6_ADDRESS); + paramh->param_length = htons(plen); memcpy(ipv6p->addr, &sin6->sin6_addr, sizeof(ipv6p->addr)); /* clear embedded scope in the address */ @@ -5141,7 +5141,10 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_ s.param_length = htons(sizeof(s) + plen); m_copyback(op_err, err_at, sizeof(s), (caddr_t)&s); err_at += sizeof(s); - phdr = sctp_get_next_param(mat, at, (struct sctp_paramhdr *)tempbuf, min(sizeof(tempbuf), plen)); + if (plen > sizeof(tempbuf)) { + plen = sizeof(tempbuf); + } + phdr = sctp_get_next_param(mat, at, (struct sctp_paramhdr *)tempbuf, plen); if (phdr == NULL) { sctp_m_freem(op_err); /* @@ -5209,7 +5212,7 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_ if (plen > sizeof(tempbuf)) { plen = sizeof(tempbuf); } - phdr = sctp_get_next_param(mat, at, (struct sctp_paramhdr *)tempbuf, min(sizeof(tempbuf), plen)); + phdr = sctp_get_next_param(mat, at, (struct sctp_paramhdr *)tempbuf, plen); if (phdr == NULL) { sctp_m_freem(op_err); /* @@ -5390,10 +5393,12 @@ sctp_are_there_new_addresses(struct sctp_association * { struct sctp_ipv4addr_param *p4, p4_buf; + if (plen != sizeof(struct sctp_ipv4addr_param)) { + return (1); + } phdr = sctp_get_next_param(in_initpkt, offset, (struct sctp_paramhdr *)&p4_buf, sizeof(p4_buf)); - if (plen != sizeof(struct sctp_ipv4addr_param) || - phdr == NULL) { + if (phdr == NULL) { return (1); } if (asoc->scope.ipv4_addr_legal) { @@ -5409,10 +5414,12 @@ sctp_are_there_new_addresses(struct sctp_association * { struct sctp_ipv6addr_param *p6, p6_buf; + if (plen != sizeof(struct sctp_ipv6addr_param)) { + return (1); + } phdr = sctp_get_next_param(in_initpkt, offset, (struct sctp_paramhdr *)&p6_buf, sizeof(p6_buf)); - if (plen != sizeof(struct sctp_ipv6addr_param) || - phdr == NULL) { + if (phdr == NULL) { return (1); } if (asoc->scope.ipv6_addr_legal) { @@ -9000,7 +9007,7 @@ sctp_send_cookie_echo(struct mbuf *m, */ int at; struct mbuf *cookie; - struct sctp_paramhdr parm, *phdr; + struct sctp_paramhdr param, *phdr; struct sctp_chunkhdr *hdr; struct sctp_tmit_chunk *chk; uint16_t ptype, plen; @@ -9010,7 +9017,7 @@ sctp_send_cookie_echo(struct mbuf *m, cookie = NULL; at = offset + sizeof(struct sctp_init_chunk); for (;;) { - phdr = sctp_get_next_param(m, at, &parm, sizeof(parm)); + phdr = sctp_get_next_param(m, at, ¶m, sizeof(param)); if (phdr == NULL) { return (-3); } Modified: head/sys/netinet/sctp_pcb.c ============================================================================== --- head/sys/netinet/sctp_pcb.c Fri Jun 23 20:49:23 2017 (r320299) +++ head/sys/netinet/sctp_pcb.c Fri Jun 23 21:01:57 2017 (r320300) @@ -2046,7 +2046,7 @@ sctp_findassociation_special_addr(struct mbuf *m, int struct sctphdr *sh, struct sctp_inpcb **inp_p, struct sctp_nets **netp, struct sockaddr *dst) { - struct sctp_paramhdr *phdr, parm_buf; + struct sctp_paramhdr *phdr, param_buf; #if defined(INET) || defined(INET6) struct sctp_tcb *stcb; uint16_t ptype; @@ -2074,7 +2074,7 @@ sctp_findassociation_special_addr(struct mbuf *m, int offset += sizeof(struct sctp_init_chunk); - phdr = sctp_get_next_param(m, offset, &parm_buf, sizeof(parm_buf)); + phdr = sctp_get_next_param(m, offset, ¶m_buf, sizeof(param_buf)); while (phdr != NULL) { /* now we must see if we want the parameter */ #if defined(INET) || defined(INET6) @@ -2088,10 +2088,10 @@ sctp_findassociation_special_addr(struct mbuf *m, int if (ptype == SCTP_IPV4_ADDRESS && plen == sizeof(struct sctp_ipv4addr_param)) { /* Get the rest of the address */ - struct sctp_ipv4addr_param ip4_parm, *p4; + struct sctp_ipv4addr_param ip4_param, *p4; phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)&ip4_parm, min(plen, sizeof(ip4_parm))); + (struct sctp_paramhdr *)&ip4_param, sizeof(ip4_param)); if (phdr == NULL) { return (NULL); } @@ -2109,10 +2109,10 @@ sctp_findassociation_special_addr(struct mbuf *m, int if (ptype == SCTP_IPV6_ADDRESS && plen == sizeof(struct sctp_ipv6addr_param)) { /* Get the rest of the address */ - struct sctp_ipv6addr_param ip6_parm, *p6; + struct sctp_ipv6addr_param ip6_param, *p6; phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)&ip6_parm, min(plen, sizeof(ip6_parm))); + (struct sctp_paramhdr *)&ip6_param, sizeof(ip6_param)); if (phdr == NULL) { return (NULL); } @@ -2127,8 +2127,8 @@ sctp_findassociation_special_addr(struct mbuf *m, int } #endif offset += SCTP_SIZE32(plen); - phdr = sctp_get_next_param(m, offset, &parm_buf, - sizeof(parm_buf)); + phdr = sctp_get_next_param(m, offset, ¶m_buf, + sizeof(param_buf)); } return (NULL); } @@ -2301,7 +2301,7 @@ sctp_findassociation_ep_asconf(struct mbuf *m, int off { struct sctp_tcb *stcb; union sctp_sockstore remote_store; - struct sctp_paramhdr parm_buf, *phdr; + struct sctp_paramhdr param_buf, *phdr; int ptype; int zero_address = 0; #ifdef INET @@ -2313,7 +2313,7 @@ sctp_findassociation_ep_asconf(struct mbuf *m, int off memset(&remote_store, 0, sizeof(remote_store)); phdr = sctp_get_next_param(m, offset + sizeof(struct sctp_asconf_chunk), - &parm_buf, sizeof(struct sctp_paramhdr)); + ¶m_buf, sizeof(struct sctp_paramhdr)); if (phdr == NULL) { SCTPDBG(SCTP_DEBUG_INPUT3, "%s: failed to get asconf lookup addr\n", __func__); @@ -2333,7 +2333,7 @@ sctp_findassociation_ep_asconf(struct mbuf *m, int off } p6 = (struct sctp_ipv6addr_param *)sctp_get_next_param(m, offset + sizeof(struct sctp_asconf_chunk), - &p6_buf.ph, sizeof(*p6)); + &p6_buf.ph, sizeof(p6_buf)); if (p6 == NULL) { SCTPDBG(SCTP_DEBUG_INPUT3, "%s: failed to get asconf v6 lookup addr\n", __func__); @@ -2360,7 +2360,7 @@ sctp_findassociation_ep_asconf(struct mbuf *m, int off } p4 = (struct sctp_ipv4addr_param *)sctp_get_next_param(m, offset + sizeof(struct sctp_asconf_chunk), - &p4_buf.ph, sizeof(*p4)); + &p4_buf.ph, sizeof(p4_buf)); if (p4 == NULL) { SCTPDBG(SCTP_DEBUG_INPUT3, "%s: failed to get asconf v4 lookup addr\n", __func__); @@ -6026,7 +6026,7 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s */ struct sctp_inpcb *inp; struct sctp_nets *net, *nnet, *net_tmp; - struct sctp_paramhdr *phdr, parm_buf; + struct sctp_paramhdr *phdr, param_buf; struct sctp_tcb *stcb_tmp; uint16_t ptype, plen; struct sockaddr *sa; @@ -6136,7 +6136,7 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s return (-4); } /* now we must go through each of the params. */ - phdr = sctp_get_next_param(m, offset, &parm_buf, sizeof(parm_buf)); + phdr = sctp_get_next_param(m, offset, ¶m_buf, sizeof(param_buf)); while (phdr) { ptype = ntohs(phdr->param_type); plen = ntohs(phdr->param_length); @@ -6374,7 +6374,7 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s } phdr = sctp_get_next_param(m, offset, (struct sctp_paramhdr *)&lstore, - min(plen, sizeof(lstore))); + plen); if (phdr == NULL) { return (-24); } @@ -6427,8 +6427,11 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s uint8_t local_store[SCTP_PARAM_BUFFER_SIZE]; int num_ent, i; + if (plen > sizeof(local_store)) { + return (-35); + } phdr = sctp_get_next_param(m, offset, - (struct sctp_paramhdr *)&local_store, min(sizeof(local_store), plen)); + (struct sctp_paramhdr *)&local_store, plen); if (phdr == NULL) { return (-25); } @@ -6475,7 +6478,7 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s } phdr = sctp_get_next_param(m, offset, (struct sctp_paramhdr *)random_store, - min(sizeof(random_store), plen)); + plen); if (phdr == NULL) return (-26); p_random = (struct sctp_auth_random *)phdr; @@ -6498,7 +6501,7 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s } phdr = sctp_get_next_param(m, offset, (struct sctp_paramhdr *)hmacs_store, - min(plen, sizeof(hmacs_store))); + plen); if (phdr == NULL) return (-28); hmacs = (struct sctp_auth_hmac_algo *)phdr; @@ -6529,7 +6532,7 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, s } phdr = sctp_get_next_param(m, offset, (struct sctp_paramhdr *)chunks_store, - min(plen, sizeof(chunks_store))); + plen); if (phdr == NULL) return (-30); chunks = (struct sctp_auth_chunk_list *)phdr; @@ -6577,8 +6580,8 @@ next_param: if (offset >= limit) { break; } - phdr = sctp_get_next_param(m, offset, &parm_buf, - sizeof(parm_buf)); + phdr = sctp_get_next_param(m, offset, ¶m_buf, + sizeof(param_buf)); } /* Now check to see if we need to purge any addresses */ TAILQ_FOREACH_SAFE(net, &stcb->asoc.nets, sctp_next, nnet) {