netdev - Re: net/sctp: out-of-bounds access in sctp_add_bind

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 25 Jan 2016 11:05:34 -0500
From:	Neil Horman <nhorman@...driver.com>
To:	Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:	Dmitry Vyukov <dvyukov@...gle.com>,
	Vlad Yasevich <vyasevich@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	linux-sctp@...r.kernel.org, netdev <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: net/sctp: out-of-bounds access in sctp_add_bind_addr

On Mon, Jan 25, 2016 at 12:48:02PM -0200, Marcelo Ricardo Leitner wrote:
> On Mon, Jan 25, 2016 at 03:42:14PM +0100, Dmitry Vyukov wrote:
> > On Mon, Jan 25, 2016 at 3:31 PM, Neil Horman <nhorman@...driver.com> wrote:
> > > On Mon, Jan 25, 2016 at 03:02:38PM +0100, Dmitry Vyukov wrote:
> > >> Hello,
> > >>
> > >> I've git the following error report while running syzkaller fuzzer:
> > >>
> > >> ==================================================================
> > >> BUG: KASAN: slab-out-of-bounds in memcpy+0x1d/0x40 at addr ffff88006c6361e8
> > >> Read of size 28 by task syz-executor/12551
> > >> =============================================================================
> > >> BUG kmalloc-16 (Not tainted): kasan: bad access detected
> > >> -----------------------------------------------------------------------------
> > >>
> > >> INFO: Allocated in sctp_setsockopt_bindx+0xd2/0x3e0 age=12 cpu=2 pid=12551
> > >> [<     inline     >] kmalloc include/linux/slab.h:468
> > >> [<      none      >] sctp_setsockopt_bindx+0xd2/0x3e0 net/sctp/socket.c:975
> > >> [<      none      >] sctp_setsockopt+0x1493/0x3630 net/sctp/socket.c:3711
> > >> [<      none      >] sock_common_setsockopt+0x97/0xd0 net/core/sock.c:2620
> > >> [<     inline     >] SYSC_setsockopt net/socket.c:1752
> > >> [<      none      >] SyS_setsockopt+0x15b/0x250 net/socket.c:1731
> > >> [<      none      >] entry_SYSCALL_64_fastpath+0x16/0x7a
> > >> arch/x86/entry/entry_64.S:185
> > >>
> > >> INFO: Slab 0xffffea0001b18d80 objects=16 used=4 fp=0xffff88006c6376e0
> > >> flags=0x5fffc0000004080
> > >> INFO: Object 0xffff88006c6361e8 @offset=488 fp=0x0000000000000002
> > >> Bytes b4 ffff88006c6361d8: 00 00 00 00 00 00 00 00 2f 98 34 88 ff ff
> > >> ff ff  ......../.4.....
> > >> Object ffff88006c6361e8: 02 00 00 00 00 00 00 00 02 00 ab 07 7f 00 00
> > >> 01  ................
> > >> CPU: 2 PID: 12551 Comm: syz-executor Tainted: G    B           4.5.0-rc1+ #278
> > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > >>  00000000ffffffff ffff880036397928 ffffffff8299a02d ffff88003e807900
> > >>  ffff88006c6361e8 ffff88006c636000 ffff880036397958 ffffffff81752814
> > >>  ffff88003e807900 ffffea0001b18d80 ffff88006c6361e8 ffff88006c6361e8
> > >>
> > >> Call Trace:
> > >>  [<ffffffff8175ad54>] __asan_loadN+0x124/0x1a0 mm/kasan/kasan.c:512
> > >>  [<ffffffff8175b2dd>] memcpy+0x1d/0x40 mm/kasan/kasan.c:297
> > >>  [<ffffffff85dcb249>] sctp_add_bind_addr+0xa9/0x270 net/sctp/bind_addr.c:162
> > >>  [<ffffffff85dcfd66>] sctp_do_bind+0x336/0x580 net/sctp/socket.c:389
> > >>  [<ffffffff85dd16ec>] sctp_bindx_add+0xac/0x1a0 net/sctp/socket.c:471
> > >>  [<ffffffff85dd5cc8>] sctp_setsockopt_bindx+0x2f8/0x3e0 net/sctp/socket.c:1010
> > >>  [<ffffffff85dde283>] sctp_setsockopt+0x1493/0x3630 net/sctp/socket.c:3711
> > >>  [<ffffffff851f5ae7>] sock_common_setsockopt+0x97/0xd0 net/core/sock.c:2620
> > >>  [<     inline     >] SYSC_setsockopt net/socket.c:1752
> > >>  [<ffffffff851f2c3b>] SyS_setsockopt+0x15b/0x250 net/socket.c:1731
> > >>  [<ffffffff863595f6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> > >> arch/x86/entry/entry_64.S:185
> > >>
> > >> Memory state around the buggy address:
> > >>  ffff88006c636080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > >>  ffff88006c636100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > >> >ffff88006c636180: fc fc fc fc fc fc fc fc fc fc fc fc fc 00 00 fc
> > >>                                                                 ^
> > >>  ffff88006c636200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > >>  ffff88006c636280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > >> ==================================================================
> > >>
> > >>
> > >> sctp_setsockopt_bindx verifies that the user-passed address has valid
> > >> len for the specified family, but then sctp_add_bind_addr copies whole
> > >> sctp_addr from there. This causes heap out-of-bounds access and can
> > >> crash kernel. Not sure if it is possible to copy out the trailing
> > >> garbage to user-space later.
> > >>
> > >
> > > It does more than that though.  sctp_setsockopt_bindx checks the following:
> > > 1) That passed addr_size is greater than zero
> > > 2) that the entire range of memory between addrs and addrs+addr_size is readable
> > > 3) That at least one address structure worth of data is available (implicit in
> > > the while (walk_size < addr_size) loop).
> > >
> > > Could one of the sockaddr_len fields in one of the addresses have been mangled
> > > so that it appeared shorter in the the while loop from (3), so that a copy of
> > > sizeof(sctp_addr in sctp_add_bind_addr overrun the allocated memory?
> > 
> > I may be missing something, but what I see is:
> > 
> > 1. we check that there is at least family:
> > if (walk_size + sizeof(sa_family_t) > addrs_size) {
> > 
> > 2. get family descriptor:
> > af = sctp_get_af_specific(sa_addr->sa_family);
> > 
> > 3. check that the address size is enough to hold the declared family:
> > if (!af || (walk_size + af->sockaddr_len) > addrs_size) {
> > 
> > 4. then we do sctp_add_bind_addr, which copies whole sctp_addr from addr:
> > 
> > int sctp_add_bind_addr(struct sctp_bind_addr *bp, union sctp_addr *new,
> > ...
> > memcpy(&addr->a, new, sizeof(*new));
> > 
> > Now imagine that the addr is ipv4 (16 or so bytes, that's what we
> > checked) and we copy 28 bytes (ipv6) from addr.
> 
> Yes, that's pretty much it I think. That memcpy should be limited to
> af->sockaddr_len, it's just that af is not readily available in that
> function.
> 
Yeah, ok, we're on the same page.  If the size of the sctp_addr struct is larger
than the size that the address family specifies, we're up the creek.  We should
augment sctp_add_bind_addr to take the family length as a parameter and either
limit the copy to the min of the sruct size and the family size

Neil

>   Marcelo
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>