d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

sctp: improve input validation

Browse Source 
Improve the handling of INIT chunks in specific szenarios and
report and appropriate error cause.
Thanks to Anatoly Korniltsev for reporting the issue for the
userland stack.

MFC after:	3 days
This commit is contained in:
Michael Tuexen 2021-01-31 23:43:15 +01:00
parent e0a0a3efcb
commit af885c57d6
1 changed files with 62 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
sys/netinet/sctp_output.c
View File

@ -5232,31 +5232,33 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_initpkt,
return (op_err);
}
static int
/*
* Given a INIT chunk, look through the parameters to verify that there
* are no new addresses.
* Return true, if there is a new address or there is a problem parsing
the parameters. Provide an optional error cause used when sending an ABORT.
* Return false, if there are no new addresses and there is no problem in
parameter processing.
*/
static bool
sctp_are_there_new_addresses(struct sctp_association *asoc,
struct mbuf *in_initpkt, int offset, struct sockaddr *src)
struct mbuf *in_initpkt, int offset, int limit, struct sockaddr *src,
struct mbuf **op_err)
{
/*
* Given a INIT packet, look through the packet to verify that there
* are NO new addresses. As we go through the parameters add reports
* of any un-understood parameters that require an error. Also we
* must return (1) to drop the packet if we see a un-understood
* parameter that tells us to drop the chunk.
*/
struct sockaddr *sa_touse;
struct sockaddr *sa;
struct sctp_paramhdr *phdr, params;
uint16_t ptype, plen;
uint8_t fnd;
struct sctp_nets *net;
int check_src;
#ifdef INET
struct sockaddr_in sin4, *sa4;
#endif
#ifdef INET6
struct sockaddr_in6 sin6, *sa6;
#endif
uint16_t ptype, plen;
bool fnd, check_src;
*op_err = NULL;
#ifdef INET
memset(&sin4, 0, sizeof(sin4));
sin4.sin_family = AF_INET;
@ -5268,19 +5270,19 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
sin6.sin6_len = sizeof(sin6);
#endif
/* First what about the src address of the pkt ? */
check_src = 0;
check_src = false;
switch (src->sa_family) {
#ifdef INET
case AF_INET:
if (asoc->scope.ipv4_addr_legal) {
check_src = 1;
check_src = true;
}
break;
#endif
#ifdef INET6
case AF_INET6:
if (asoc->scope.ipv6_addr_legal) {
check_src = 1;
check_src = true;
}
break;
#endif
@ -5289,7 +5291,7 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
break;
}
if (check_src) {
fnd = 0;
fnd = false;
TAILQ_FOREACH(net, &asoc->nets, sctp_next) {
sa = (struct sockaddr *)&net->ro._l_addr;
if (sa->sa_family == src->sa_family) {
@ -5300,7 +5302,7 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
sa4 = (struct sockaddr_in *)sa;
src4 = (struct sockaddr_in *)src;
if (sa4->sin_addr.s_addr == src4->sin_addr.s_addr) {
fnd = 1;
fnd = true;
break;
}
}
@ -5312,16 +5314,22 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
sa6 = (struct sockaddr_in6 *)sa;
src6 = (struct sockaddr_in6 *)src;
if (SCTP6_ARE_ADDR_EQUAL(sa6, src6)) {
fnd = 1;
fnd = true;
break;
}
}
#endif
}
}
if (fnd == 0) {
/* New address added! no need to look further. */
return (1);
if (!fnd) {
/*
* If sending an ABORT in case of an additional
* address, don't use the new address error cause.
* This looks no different than if no listener was
* present.
*/
*op_err = sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code), "Address added");
return (true);
}
}
/* Ok so far lets munge through the rest of the packet */
@ -5331,6 +5339,14 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
sa_touse = NULL;
ptype = ntohs(phdr->param_type);
plen = ntohs(phdr->param_length);
if (offset + plen > limit) {
*op_err = sctp_generate_cause(SCTP_CAUSE_PROTOCOL_VIOLATION, "Partial parameter");
return (true);
}
if (plen < sizeof(struct sctp_paramhdr)) {
*op_err = sctp_generate_cause(SCTP_CAUSE_PROTOCOL_VIOLATION, "Parameter length too small");
return (true);
}
switch (ptype) {
#ifdef INET
case SCTP_IPV4_ADDRESS:
@ -5338,12 +5354,14 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
struct sctp_ipv4addr_param *p4, p4_buf;
if (plen != sizeof(struct sctp_ipv4addr_param)) {
return (1);
*op_err = sctp_generate_cause(SCTP_CAUSE_PROTOCOL_VIOLATION, "Parameter length illegal");
return (true);
}
phdr = sctp_get_next_param(in_initpkt, offset,
(struct sctp_paramhdr *)&p4_buf, sizeof(p4_buf));
if (phdr == NULL) {
return (1);
*op_err = sctp_generate_cause(SCTP_CAUSE_PROTOCOL_VIOLATION, "");
return (true);
}
if (asoc->scope.ipv4_addr_legal) {
p4 = (struct sctp_ipv4addr_param *)phdr;
@ -5359,12 +5377,14 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
struct sctp_ipv6addr_param *p6, p6_buf;
if (plen != sizeof(struct sctp_ipv6addr_param)) {
return (1);
*op_err = sctp_generate_cause(SCTP_CAUSE_PROTOCOL_VIOLATION, "Parameter length illegal");
return (true);
}
phdr = sctp_get_next_param(in_initpkt, offset,
(struct sctp_paramhdr *)&p6_buf, sizeof(p6_buf));
if (phdr == NULL) {
return (1);
*op_err = sctp_generate_cause(SCTP_CAUSE_PROTOCOL_VIOLATION, "");
return (true);
}
if (asoc->scope.ipv6_addr_legal) {
p6 = (struct sctp_ipv6addr_param *)phdr;
@ -5381,7 +5401,7 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
}
if (sa_touse) {
/* ok, sa_touse points to one to check */
fnd = 0;
fnd = false;
TAILQ_FOREACH(net, &asoc->nets, sctp_next) {
sa = (struct sockaddr *)&net->ro._l_addr;
if (sa->sa_family != sa_touse->sa_family) {
@ -5392,7 +5412,7 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
sa4 = (struct sockaddr_in *)sa;
if (sa4->sin_addr.s_addr ==
sin4.sin_addr.s_addr) {
fnd = 1;
fnd = true;
break;
}
}
@ -5402,21 +5422,31 @@ sctp_are_there_new_addresses(struct sctp_association *asoc,
sa6 = (struct sockaddr_in6 *)sa;
if (SCTP6_ARE_ADDR_EQUAL(
sa6, &sin6)) {
fnd = 1;
fnd = true;
break;
}
}
#endif
}
if (!fnd) {
/* New addr added! no need to look further */
return (1);
/*
* If sending an ABORT in case of an
* additional address, don't use the new
* address error cause. This looks no
* different than if no listener was
* present.
*/
*op_err = sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code), "Address added");
return (true);
}
}
offset += SCTP_SIZE32(plen);
if (offset >= limit) {
break;
}
phdr = sctp_get_next_param(in_initpkt, offset, &params, sizeof(params));
}
return (0);
return (false);
}
/*
@ -5472,17 +5502,11 @@ sctp_send_initiate_ack(struct sctp_inpcb *inp, struct sctp_tcb *stcb,
}
if ((asoc != NULL) &&
(SCTP_GET_STATE(stcb) != SCTP_STATE_COOKIE_WAIT)) {
if (sctp_are_there_new_addresses(asoc, init_pkt, offset, src)) {
if (sctp_are_there_new_addresses(asoc, init_pkt, offset, offset + ntohs(init_chk->ch.chunk_length), src, &op_err)) {
/*
* new addresses, out of here in non-cookie-wait
* states
*
* Send an ABORT, without the new address error
* cause. This looks no different than if no
* listener was present.
*/
op_err = sctp_generate_cause(SCTP_BASE_SYSCTL(sctp_diag_info_code),
"Address added");
sctp_send_abort(init_pkt, iphlen, src, dst, sh, 0, op_err,
mflowtype, mflowid, inp->fibnum,
vrf_id, port);