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@xxxxxxxxxx> > Signed-off-by: Vlad Yasevich <vyasevich@xxxxxxxxx> Acked-by: Neil Horman <nhorman@xxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html