[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 10 Oct 2014 10:48:07 -0400
From: Neil Horman <nhorman@...driver.com>
To: Daniel Borkmann <dborkman@...hat.com>
Cc: davem@...emloft.net, linux-sctp@...r.kernel.org,
netdev@...r.kernel.org, Vlad Yasevich <vyasevich@...il.com>
Subject: Re: [PATCH net 1/3] net: sctp: fix skb_over_panic when receiving
malformed ASCONF chunks
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@...hat.com>
> Signed-off-by: Vlad Yasevich <vyasevich@...il.com>
Acked-by: Neil Horman <nhorman@...driver.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists