Re: [PATCH] netrom: fix info leak via getsockname()
flat view
From: Ben Hutchings <hidden>
Date: 2012-10-14 23:11:40
On Sun, 2012-10-14 at 20:23 +0200, Clément Lecigne wrote:
quoted hunk ↗ jump to hunk
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 <redacted>--- a/net/netrom/af_netrom.c +++ b/net/netrom/af_netrom.c@@ -838,6 +838,8 @@ static int nr_getname(struct socket *sock, structsockaddr *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.