From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Date: Fri, 10 Oct 2014 14:48:07 +0000 Subject: Re: [PATCH net 1/3] net: sctp: fix skb_over_panic when receiving malformed ASCONF chunks Message-Id: <20141010144806.GD19499@hmsreliant.think-freely.org> List-Id: References: <1412888133-833-1-git-send-email-dborkman@redhat.com> <1412888133-833-2-git-send-email-dborkman@redhat.com> In-Reply-To: <1412888133-833-2-git-send-email-dborkman@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Daniel Borkmann Cc: davem@davemloft.net, linux-sctp@vger.kernel.org, netdev@vger.kernel.org, Vlad Yasevich 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: > ------------[ cut here ]------------ > kernel BUG at net/core/skbuff.c:129! > [...] > Call Trace: > > [] skb_put+0x5c/0x70 > [] sctp_addto_chunk+0x63/0xd0 [sctp] > [] sctp_process_asconf+0x1af/0x540 [sctp] > [] ? _read_unlock_bh+0x15/0x20 > [] sctp_sf_do_asconf+0x168/0x240 [sctp] > [] sctp_do_sm+0x71/0x1210 [sctp] > [] ? fib_rules_lookup+0xad/0xf0 > [] ? sctp_cmp_addr_exact+0x32/0x40 [sctp] > [] sctp_assoc_bh_rcv+0xd3/0x180 [sctp] > [] sctp_inq_push+0x56/0x80 [sctp] > [] sctp_rcv+0x982/0xa10 [sctp] > [] ? ipt_local_in_hook+0x23/0x28 [iptable_filter] > [] ? nf_iterate+0x69/0xb0 > [] ? ip_local_deliver_finish+0x0/0x2d0 > [] ? nf_hook_slow+0x76/0x120 > [] ? ip_local_deliver_finish+0x0/0x2d0 > [] ip_local_deliver_finish+0xdd/0x2d0 > [] ip_local_deliver+0x98/0xa0 > [] ip_rcv_finish+0x12d/0x440 > [] ip_rcv+0x275/0x350 > [] __netif_receive_skb+0x4ab/0x750 > [] 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 > Signed-off-by: Vlad Yasevich Acked-by: Neil Horman