Message ID | 1412888133-833-2-git-send-email-dborkman@redhat.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On 10/09/2014 16:55, Daniel Borkmann wrote: > Commit 6f4c618ddb0 ("SCTP : Add paramters validity check for > ASCONF chunk") added basic verification of ASCONF chunks, however, > it is still possible to remotely crash a server by sending a > special crafted ASCONF chunk, even up to pre 2.6.12 kernels: > > skb_over_panic: text:ffffffffa01ea1c3 len:31056 put:30768 > head:ffff88011bd81800 data:ffff88011bd81800 tail:0x7950 > end:0x440 dev:<NULL> > ------------[ cut here ]------------ > kernel BUG at net/core/skbuff.c:129! > [...] > Call Trace: > <IRQ> > [<ffffffff8144fb1c>] skb_put+0x5c/0x70 > [<ffffffffa01ea1c3>] sctp_addto_chunk+0x63/0xd0 [sctp] [snip] > > This can be triggered e.g., through a simple scripted nmap > connection scan injecting the chunk after the handshake, for > example, ... > > -------------- INIT[ASCONF; ASCONF_ACK] -------------> > <----------- INIT-ACK[ASCONF; ASCONF_ACK] ------------ > -------------------- COOKIE-ECHO --------------------> > <-------------------- COOKIE-ACK --------------------- > ------------------ ASCONF; UNKNOWN ------------------> > > ... where ASCONF chunk of length 280 contains 2 parameters ... > > 1) Add IP address parameter (param length: 16) > 2) Add/del IP address parameter (param length: 255) > > ... followed by an UNKNOWN chunk of e.g. 4 bytes. Here, the > Address Parameter in the ASCONF chunk is even missing, too. > This is just an example and similarly-crafted ASCONF chunks > could be used just as well. If I am reading correctly, this crash can only be triggered by actually getting through the SCTP handshake, then sending this specially-crafted ASCONF chunk? Meaning a blind nmap scan using this tactic against a random netblock wouldn't just randomly knock servers offline? This would seem to reduce the attack surface a quite bit by requiring the remote endpoint to actually respond. Is there a CVE # for this? Thanks!,
On Thu, Oct 09, 2014 at 10:55:31PM +0200, Daniel Borkmann wrote: > Commit 6f4c618ddb0 ("SCTP : Add paramters validity check for > ASCONF chunk") added basic verification of ASCONF chunks, however, > it is still possible to remotely crash a server by sending a > special crafted ASCONF chunk, even up to pre 2.6.12 kernels: > > skb_over_panic: text:ffffffffa01ea1c3 len:31056 put:30768 > head:ffff88011bd81800 data:ffff88011bd81800 tail:0x7950 > end:0x440 dev:<NULL> > ------------[ cut here ]------------ > kernel BUG at net/core/skbuff.c:129! > [...] > Call Trace: > <IRQ> > [<ffffffff8144fb1c>] skb_put+0x5c/0x70 > [<ffffffffa01ea1c3>] sctp_addto_chunk+0x63/0xd0 [sctp] > [<ffffffffa01eadaf>] sctp_process_asconf+0x1af/0x540 [sctp] > [<ffffffff8152d025>] ? _read_unlock_bh+0x15/0x20 > [<ffffffffa01e0038>] sctp_sf_do_asconf+0x168/0x240 [sctp] > [<ffffffffa01e3751>] sctp_do_sm+0x71/0x1210 [sctp] > [<ffffffff8147645d>] ? fib_rules_lookup+0xad/0xf0 > [<ffffffffa01e6b22>] ? sctp_cmp_addr_exact+0x32/0x40 [sctp] > [<ffffffffa01e8393>] sctp_assoc_bh_rcv+0xd3/0x180 [sctp] > [<ffffffffa01ee986>] sctp_inq_push+0x56/0x80 [sctp] > [<ffffffffa01fcc42>] sctp_rcv+0x982/0xa10 [sctp] > [<ffffffffa01d5123>] ? ipt_local_in_hook+0x23/0x28 [iptable_filter] > [<ffffffff8148bdc9>] ? nf_iterate+0x69/0xb0 > [<ffffffff81496d10>] ? ip_local_deliver_finish+0x0/0x2d0 > [<ffffffff8148bf86>] ? nf_hook_slow+0x76/0x120 > [<ffffffff81496d10>] ? ip_local_deliver_finish+0x0/0x2d0 > [<ffffffff81496ded>] ip_local_deliver_finish+0xdd/0x2d0 > [<ffffffff81497078>] ip_local_deliver+0x98/0xa0 > [<ffffffff8149653d>] ip_rcv_finish+0x12d/0x440 > [<ffffffff81496ac5>] ip_rcv+0x275/0x350 > [<ffffffff8145c88b>] __netif_receive_skb+0x4ab/0x750 > [<ffffffff81460588>] netif_receive_skb+0x58/0x60 > > This can be triggered e.g., through a simple scripted nmap > connection scan injecting the chunk after the handshake, for > example, ... > > -------------- INIT[ASCONF; ASCONF_ACK] -------------> > <----------- INIT-ACK[ASCONF; ASCONF_ACK] ------------ > -------------------- COOKIE-ECHO --------------------> > <-------------------- COOKIE-ACK --------------------- > ------------------ ASCONF; UNKNOWN ------------------> > > ... where ASCONF chunk of length 280 contains 2 parameters ... > > 1) Add IP address parameter (param length: 16) > 2) Add/del IP address parameter (param length: 255) > > ... followed by an UNKNOWN chunk of e.g. 4 bytes. Here, the > Address Parameter in the ASCONF chunk is even missing, too. > This is just an example and similarly-crafted ASCONF chunks > could be used just as well. > > The ASCONF chunk passes through sctp_verify_asconf() as all > parameters passed sanity checks, and after walking, we ended > up successfully at the chunk end boundary, and thus may invoke > sctp_process_asconf(). Parameter walking is done with > WORD_ROUND() to take padding into account. > > In sctp_process_asconf()'s TLV processing, we may fail in > sctp_process_asconf_param() e.g., due to removal of the IP > address that is also the source address of the packet containing > the ASCONF chunk, and thus we need to add all TLVs after the > failure to our ASCONF response to remote via helper function > sctp_add_asconf_response(), which basically invokes a > sctp_addto_chunk() adding the error parameters to the given > skb. > > When walking to the next parameter this time, we proceed > with ... > > length = ntohs(asconf_param->param_hdr.length); > asconf_param = (void *)asconf_param + length; > > ... instead of the WORD_ROUND()'ed length, thus resulting here > in an off-by-one that leads to reading the follow-up garbage > parameter length of 12336, and thus throwing an skb_over_panic > for the reply when trying to sctp_addto_chunk() next time, > which implicitly calls the skb_put() with that length. > > Fix it by using sctp_walk_params() [ which is also used in > INIT parameter processing ] macro in the verification *and* > in ASCONF processing: it will make sure we don't spill over, > that we walk parameters WORD_ROUND()'ed. Moreover, we're being > more defensive and guard against unknown parameter types and > missized addresses. > > Joint work with Vlad Yasevich. > > Fixes: b896b82be4ae ("[SCTP] ADDIP: Support for processing incoming ASCONF_ACK chunks.") > Signed-off-by: Daniel Borkmann <dborkman@redhat.com> > Signed-off-by: Vlad Yasevich <vyasevich@gmail.com> Acked-by: Neil Horman <nhorman@tuxdriver.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/2014 12:04 PM, Joshua Kinard wrote: ... > If I am reading correctly, this crash can only be triggered by actually getting > through the SCTP handshake, then sending this specially-crafted ASCONF chunk? > Meaning a blind nmap scan using this tactic against a random netblock wouldn't > just randomly knock servers offline? This would seem to reduce the attack > surface a quite bit by requiring the remote endpoint to actually respond. Sorry, have been on travel almost whole day ... yes, handshake has to be completed before that. So a scan/probe would need to establish a connection first and ASCONF would need to be supported. > Is there a CVE # for this? CVE-2014-3673 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h index 7f4eeb3..72a31db 100644 --- a/include/net/sctp/sm.h +++ b/include/net/sctp/sm.h @@ -248,9 +248,9 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *, int, __be16); struct sctp_chunk *sctp_make_asconf_set_prim(struct sctp_association *asoc, union sctp_addr *addr); -int sctp_verify_asconf(const struct sctp_association *asoc, - struct sctp_paramhdr *param_hdr, void *chunk_end, - struct sctp_paramhdr **errp); +bool sctp_verify_asconf(const struct sctp_association *asoc, + struct sctp_chunk *chunk, bool addr_param_needed, + struct sctp_paramhdr **errp); struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc, struct sctp_chunk *asconf); int sctp_process_asconf_ack(struct sctp_association *asoc, diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index ae0e616..ab734be 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -3110,50 +3110,63 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc, return SCTP_ERROR_NO_ERROR; } -/* Verify the ASCONF packet before we process it. */ -int sctp_verify_asconf(const struct sctp_association *asoc, - struct sctp_paramhdr *param_hdr, void *chunk_end, - struct sctp_paramhdr **errp) { - sctp_addip_param_t *asconf_param; +/* Verify the ASCONF packet before we process it. */ +bool sctp_verify_asconf(const struct sctp_association *asoc, + struct sctp_chunk *chunk, bool addr_param_needed, + struct sctp_paramhdr **errp) +{ + sctp_addip_chunk_t *addip = (sctp_addip_chunk_t *) chunk->chunk_hdr; union sctp_params param; - int length, plen; - - param.v = (sctp_paramhdr_t *) param_hdr; - while (param.v <= chunk_end - sizeof(sctp_paramhdr_t)) { - length = ntohs(param.p->length); - *errp = param.p; + bool addr_param_seen = false; - if (param.v > chunk_end - length || - length < sizeof(sctp_paramhdr_t)) - return 0; + sctp_walk_params(param, addip, addip_hdr.params) { + size_t length = ntohs(param.p->length); + *errp = param.p; switch (param.p->type) { + case SCTP_PARAM_ERR_CAUSE: + break; + case SCTP_PARAM_IPV4_ADDRESS: + if (length != sizeof(sctp_ipv4addr_param_t)) + return false; + addr_param_seen = true; + break; + case SCTP_PARAM_IPV6_ADDRESS: + if (length != sizeof(sctp_ipv6addr_param_t)) + return false; + addr_param_seen = true; + break; case SCTP_PARAM_ADD_IP: case SCTP_PARAM_DEL_IP: case SCTP_PARAM_SET_PRIMARY: - asconf_param = (sctp_addip_param_t *)param.v; - plen = ntohs(asconf_param->param_hdr.length); - if (plen < sizeof(sctp_addip_param_t) + - sizeof(sctp_paramhdr_t)) - return 0; + /* In ASCONF chunks, these need to be first. */ + if (addr_param_needed && !addr_param_seen) + return false; + length = ntohs(param.addip->param_hdr.length); + if (length < sizeof(sctp_addip_param_t) + + sizeof(sctp_paramhdr_t)) + return false; break; case SCTP_PARAM_SUCCESS_REPORT: case SCTP_PARAM_ADAPTATION_LAYER_IND: if (length != sizeof(sctp_addip_param_t)) - return 0; - + return false; break; default: - break; + /* This is unkown to us, reject! */ + return false; } - - param.v += WORD_ROUND(length); } - if (param.v != chunk_end) - return 0; + /* Remaining sanity checks. */ + if (addr_param_needed && !addr_param_seen) + return false; + if (!addr_param_needed && addr_param_seen) + return false; + if (param.v != chunk->chunk_end) + return false; - return 1; + return true; } /* Process an incoming ASCONF chunk with the next expected serial no. and @@ -3162,16 +3175,17 @@ int sctp_verify_asconf(const struct sctp_association *asoc, struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc, struct sctp_chunk *asconf) { + sctp_addip_chunk_t *addip = (sctp_addip_chunk_t *) asconf->chunk_hdr; + bool all_param_pass = true; + union sctp_params param; sctp_addiphdr_t *hdr; union sctp_addr_param *addr_param; sctp_addip_param_t *asconf_param; struct sctp_chunk *asconf_ack; - __be16 err_code; int length = 0; int chunk_len; __u32 serial; - int all_param_pass = 1; chunk_len = ntohs(asconf->chunk_hdr->length) - sizeof(sctp_chunkhdr_t); hdr = (sctp_addiphdr_t *)asconf->skb->data; @@ -3199,9 +3213,14 @@ struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc, goto done; /* Process the TLVs contained within the ASCONF chunk. */ - while (chunk_len > 0) { + sctp_walk_params(param, addip, addip_hdr.params) { + /* Skip preceeding address parameters. */ + if (param.p->type == SCTP_PARAM_IPV4_ADDRESS || + param.p->type == SCTP_PARAM_IPV6_ADDRESS) + continue; + err_code = sctp_process_asconf_param(asoc, asconf, - asconf_param); + param.addip); /* ADDIP 4.1 A7) * If an error response is received for a TLV parameter, * all TLVs with no response before the failed TLV are @@ -3209,28 +3228,20 @@ struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc, * the failed response are considered unsuccessful unless * a specific success indication is present for the parameter. */ - if (SCTP_ERROR_NO_ERROR != err_code) - all_param_pass = 0; - + if (err_code != SCTP_ERROR_NO_ERROR) + all_param_pass = false; if (!all_param_pass) - sctp_add_asconf_response(asconf_ack, - asconf_param->crr_id, err_code, - asconf_param); + sctp_add_asconf_response(asconf_ack, param.addip->crr_id, + err_code, param.addip); /* ADDIP 4.3 D11) When an endpoint receiving an ASCONF to add * an IP address sends an 'Out of Resource' in its response, it * MUST also fail any subsequent add or delete requests bundled * in the ASCONF. */ - if (SCTP_ERROR_RSRC_LOW == err_code) + if (err_code == SCTP_ERROR_RSRC_LOW) goto done; - - /* Move to the next ASCONF param. */ - length = ntohs(asconf_param->param_hdr.length); - asconf_param = (void *)asconf_param + length; - chunk_len -= length; } - done: asoc->peer.addip_serial++; diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index c8f6063..bdea3df 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -3591,9 +3591,7 @@ sctp_disposition_t sctp_sf_do_asconf(struct net *net, struct sctp_chunk *asconf_ack = NULL; struct sctp_paramhdr *err_param = NULL; sctp_addiphdr_t *hdr; - union sctp_addr_param *addr_param; __u32 serial; - int length; if (!sctp_vtag_verify(chunk, asoc)) { sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_BAD_TAG, @@ -3618,17 +3616,8 @@ sctp_disposition_t sctp_sf_do_asconf(struct net *net, hdr = (sctp_addiphdr_t *)chunk->skb->data; serial = ntohl(hdr->serial); - addr_param = (union sctp_addr_param *)hdr->params; - length = ntohs(addr_param->p.length); - if (length < sizeof(sctp_paramhdr_t)) - return sctp_sf_violation_paramlen(net, ep, asoc, type, arg, - (void *)addr_param, commands); - /* Verify the ASCONF chunk before processing it. */ - if (!sctp_verify_asconf(asoc, - (sctp_paramhdr_t *)((void *)addr_param + length), - (void *)chunk->chunk_end, - &err_param)) + if (!sctp_verify_asconf(asoc, chunk, true, &err_param)) return sctp_sf_violation_paramlen(net, ep, asoc, type, arg, (void *)err_param, commands); @@ -3745,10 +3734,7 @@ sctp_disposition_t sctp_sf_do_asconf_ack(struct net *net, rcvd_serial = ntohl(addip_hdr->serial); /* Verify the ASCONF-ACK chunk before processing it. */ - if (!sctp_verify_asconf(asoc, - (sctp_paramhdr_t *)addip_hdr->params, - (void *)asconf_ack->chunk_end, - &err_param)) + if (!sctp_verify_asconf(asoc, asconf_ack, false, &err_param)) return sctp_sf_violation_paramlen(net, ep, asoc, type, arg, (void *)err_param, commands);