Message ID | 20110525155918.GA27869@suse.de |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Marcus Meissner <meissner@suse.de> Date: Wed, 25 May 2011 17:59:18 +0200 > @@ -465,6 +465,9 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) > if (addr_len < sizeof(struct sockaddr_in)) > goto out; > > + if (addr->sin_family != AF_INET) > + goto out; > + > chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr); Since we haven't been validating the sin_family field for 18+ years, the chance to break some applications is very real. But I think it's more important to fix this (and force any broken apps to set sin_family correctly). So I will apply this, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 1 Jun 2011 at 21:03, David Miller wrote: > Since we haven't been validating the sin_family field for 18+ years, > the chance to break some applications is very real. > > But I think it's more important to fix this (and force any broken > apps to set sin_family correctly). So I will apply this, thanks. I think a corresponding check should also go into inet6_bind() in net/ipv6/af_inet6.c . cu Reinhard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index cc14631..9c19260 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -465,6 +465,9 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) if (addr_len < sizeof(struct sockaddr_in)) goto out; + if (addr->sin_family != AF_INET) + goto out; + chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr); /* Not specified by any standard per-se, however it breaks too
Hi, By chance Reinhard Max spotted an interesting flaw in Linux bind(2)... If you create a IPv4 socket and then incorrectly bind(2) to a IPv6 address (which you got from getaddrinfo(3) or similar), the socket will be bound to INADDR_ANY. The reason is that the kernel just takes the sockaddr_in6 struct and evaluates it as a sockaddr_in struct, with the port being OK, but the IPv4 sin_addr overlaying the IPv6 sin6_flowinfo field. As the sin6_flowinfo field is usually 0, your service can end up listening to the world. A testprogram that you can strace is attached, run netstat -apn |grep 12345 afterwards to see it binds 0.0.0.0:12345. Perhaps add a check like the one below? (untested) Or use if (addr->sin_family == AF_INET6) to just catch the IPv6 case? Ciao, Marcus Subject: [PATCH] net/ipv4: Check for mistakenly passed in non-IPv4 address Hi, Check against mistakenly passing in IPv6 addresses (which would result in an INADDR_ANY bind) or similar incompatible sockaddrs. Ciao, Marcus Signed-off-by: Marcus Meissner <meissner@suse.de> Cc: Reinhard Max <max@suse.de> --- net/ipv4/af_inet.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)