Skip to content

Commit

Permalink
Improve the handling of state cookie parameters in INIT-ACK chunks.
Browse files Browse the repository at this point in the history 
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
sctplab/usrsctp#351
and
sctplab/usrsctp#352
  • Loading branch information
@tuexen
tuexen committed Sep 1, 2019
1 parent a2434dc commit b9f8104
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 52 deletions.
74 changes: 34 additions & 40 deletions netinet/sctp_input.c
View file
Expand Up @@ -34,7 +34,7 @@

#ifdef __FreeBSD__
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: head/sys/netinet/sctp_input.c 349986 2019-07-14 12:04:39Z tuexen $");
__FBSDID("$FreeBSD: head/sys/netinet/sctp_input.c 351654 2019-09-01 10:09:53Z tuexen $");
#endif

#include <netinet/sctp_os.h>
Expand Down Expand Up @@ -471,22 +471,50 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int offset,
{
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,
#if defined(__FreeBSD__)
mflowtype, mflowid,
#endif
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 */
Expand Down Expand Up @@ -578,42 +606,8 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int offset,
}
}
#endif
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,
#if defined(__FreeBSD__)
mflowtype, mflowid,
#endif
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
Expand Down
43 changes: 34 additions & 9 deletions netinet/sctp_output.c
View file
Expand Up @@ -34,7 +34,7 @@

#ifdef __FreeBSD__
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: head/sys/netinet/sctp_output.c 350625 2019-08-06 08:33:21Z tuexen $");
__FBSDID("$FreeBSD: head/sys/netinet/sctp_output.c 351654 2019-09-01 10:09:53Z tuexen $");
#endif

#include <netinet/sctp_os.h>
Expand Down Expand Up @@ -5358,7 +5358,10 @@ sctp_send_initiate(struct sctp_inpcb *inp, struct sctp_tcb *stcb, int so_locked

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
Expand All @@ -5381,6 +5384,9 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_initpkt,
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;
Expand Down Expand Up @@ -5409,12 +5415,17 @@ sctp_arethere_unrecognized_parameters(struct mbuf *in_initpkt,
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:
Expand Down Expand Up @@ -5990,8 +6001,10 @@ sctp_send_initiate_ack(struct sctp_inpcb *inp, struct sctp_tcb *stcb,
}
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);
(offset + sizeof(struct sctp_init_chunk)),
&abort_flag,
(struct sctp_chunkhdr *)init_chk,
&nat_friendly, NULL);
if (abort_flag) {
do_a_abort:
if (op_err == NULL) {
Expand Down Expand Up @@ -9511,7 +9524,7 @@ sctp_queue_op_err(struct sctp_tcb *stcb, struct mbuf *op_err)

int
sctp_send_cookie_echo(struct mbuf *m,
int offset,
int offset, int limit,
struct sctp_tcb *stcb,
struct sctp_nets *net)
{
Expand All @@ -9537,18 +9550,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);
Expand All @@ -9574,7 +9599,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;
Expand Down
6 changes: 3 additions & 3 deletions netinet/sctp_output.h
View file
Expand Up @@ -34,7 +34,7 @@

#ifdef __FreeBSD__
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: head/sys/netinet/sctp_output.h 323861 2017-09-21 11:56:31Z tuexen $");
__FBSDID("$FreeBSD: head/sys/netinet/sctp_output.h 351654 2019-09-01 10:09:53Z tuexen $");
#endif

#ifndef _NETINET_SCTP_OUTPUT_H_
Expand Down Expand Up @@ -97,11 +97,11 @@ sctp_send_initiate_ack(struct sctp_inpcb *, struct sctp_tcb *,

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 *);
Expand Down

0 comments on commit b9f8104

Please sign in to comment.