netdev - Re: [PATCH] netrom: fix info leak via getsockname()

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]

Message-ID: <1350256293.26833.28.camel@deadeye.wl.decadent.org.uk>
Date:	Mon, 15 Oct 2012 00:11:33 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Clément Lecigne <clemun@...il.com>
CC:	<netdev@...r.kernel.org>
Subject: Re: [PATCH] netrom: fix info leak via getsockname()

On Sun, 2012-10-14 at 20:23 +0200, Clément Lecigne wrote:
> The following patch fixes a 3 bytes info leak via getsockname() on
> AF_NETROM socket.
> 
> Details:
> 
> typedef struct {
> 	char		ax25_call[7];	/* 6 call + SSID (shifted ascii!) */
> } ax25_address;
> 
> struct sockaddr_ax25 {
> 	__kernel_sa_family_t sax25_family;
> 	ax25_address	sax25_call;
> 	int		sax25_ndigis;
> 	/* Digipeater ax25_address sets follow */
> };
> 
> After compilation, gcc will add 3 padding bytes after sax25_call to
> align sax25_ndigis and nr_getname does not clear these padding bytes
> before returning to userland, poc attached.
> 
> Signed-off-by: Clement Lecigne <clemun@...il.com>
> 
> --- a/net/netrom/af_netrom.c
> +++ b/net/netrom/af_netrom.c
> @@ -838,6 +838,8 @@ static int nr_getname(struct socket *sock, struct
> sockaddr *uaddr,
>         struct sock *sk = sock->sk;
>         struct nr_sock *nr = nr_sk(sk);
> 
> +    memset(sax, 0, sizeof(struct sockaddr_ax25));

This is not indented correctly.

>         lock_sock(sk);
>         if (peer != 0) {
>                 if (sk->sk_state != TCP_ESTABLISHED) {

On some architectures, structures have a minimum alignment of 4, so
there are 2 bytes of padding before sax25_call and 1 byte within it.
nr_getname() copies complete ax25_address structures, but nr_listen()
does not initialise the 1 byte of padding in user_addr:

		memset(&nr_sk(sk)->user_addr, 0, AX25_ADDR_LEN);

So I think you should address that too.  It appears that other
ax25_address structures are originally copied from userland and
therefore don't leak information, but I'm not sure.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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