Add missing sockaddr length and family validation to various protocols

Several protocol methods take a sockaddr as input.  In some cases the
sockaddr lengths were not being validated, or were validated after some
out-of-bounds accesses could occur.  Add requisite checking to various
protocol entry points, and convert some existing checks to assertions
where appropriate.

Reported by:	syzkaller+KASAN
Reviewed by:	tuexen, melifaro
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D29519
This commit is contained in:
Mark Johnston 2021-05-03 12:51:04 -04:00
parent a3c7da3d08
commit f161d294b9
15 changed files with 203 additions and 70 deletions

View file

@ -300,6 +300,7 @@ hvs_addr_set(struct sockaddr_hvs *addr, unsigned int port)
{
memset(addr, 0, sizeof(*addr));
addr->sa_family = AF_HYPERV;
addr->sa_len = sizeof(*addr);
addr->hvs_port = port;
}
@ -430,6 +431,12 @@ hvs_trans_bind(struct socket *so, struct sockaddr *addr, struct thread *td)
__func__, sa->sa_family);
return (EAFNOSUPPORT);
}
if (sa->sa_len != sizeof(*sa)) {
HVSOCK_DBG(HVSOCK_DBG_ERR,
"%s: Not supported, sa_len is %u\n",
__func__, sa->sa_len);
return (EINVAL);
}
HVSOCK_DBG(HVSOCK_DBG_VERBOSE,
"%s: binding port = 0x%x\n", __func__, sa->hvs_port);
@ -521,6 +528,8 @@ hvs_trans_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
return (EINVAL);
if (raddr->sa_family != AF_HYPERV)
return (EAFNOSUPPORT);
if (raddr->sa_len != sizeof(*raddr))
return (EINVAL);
mtx_lock(&hvs_trans_socks_mtx);
if (so->so_state &

View file

@ -240,11 +240,16 @@ ngc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
goto release;
}
if (sap->sg_len > NG_NODESIZ + offsetof(struct sockaddr_ng, sg_data)) {
error = EINVAL;
goto release;
}
/*
* Allocate an expendable buffer for the path, chop off
* the sockaddr header, and make sure it's NUL terminated.
*/
len = sap->sg_len - 2;
len = sap->sg_len - offsetof(struct sockaddr_ng, sg_data);
path = malloc(len + 1, M_NETGRAPH_PATH, M_WAITOK);
bcopy(sap->sg_data, path, len);
path[len] = '\0';
@ -422,10 +427,16 @@ ngd_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
goto release;
}
if (sap == NULL)
if (sap == NULL) {
len = 0; /* Make compiler happy. */
else
len = sap->sg_len - 2;
} else {
if (sap->sg_len > NG_NODESIZ +
offsetof(struct sockaddr_ng, sg_data)) {
error = EINVAL;
goto release;
}
len = sap->sg_len - offsetof(struct sockaddr_ng, sg_data);
}
/*
* If the user used any of these ways to not specify an address

View file

@ -654,6 +654,10 @@ in_pcbbind(struct inpcb *inp, struct sockaddr *nam, struct ucred *cred)
{
int anonport, error;
KASSERT(nam == NULL || nam->sa_family == AF_INET,
("%s: invalid address family for %p", __func__, nam));
KASSERT(nam == NULL || nam->sa_len == sizeof(struct sockaddr_in),
("%s: invalid address length for %p", __func__, nam));
INP_WLOCK_ASSERT(inp);
INP_HASH_WLOCK_ASSERT(inp->inp_pcbinfo);
@ -940,16 +944,11 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam, in_addr_t *laddrp,
return (error);
} else {
sin = (struct sockaddr_in *)nam;
if (nam->sa_len != sizeof (*sin))
return (EINVAL);
#ifdef notdef
/*
* We should check the family, but old programs
* incorrectly fail to initialize it.
*/
if (sin->sin_family != AF_INET)
return (EAFNOSUPPORT);
#endif
KASSERT(sin->sin_family == AF_INET,
("%s: invalid family for address %p", __func__, sin));
KASSERT(sin->sin_len == sizeof(*sin),
("%s: invalid length for address %p", __func__, sin));
error = prison_local_ip4(cred, &sin->sin_addr);
if (error)
return (error);
@ -1372,6 +1371,11 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr *nam,
u_short lport, fport;
int error;
KASSERT(sin->sin_family == AF_INET,
("%s: invalid address family for %p", __func__, sin));
KASSERT(sin->sin_len == sizeof(*sin),
("%s: invalid address length for %p", __func__, sin));
/*
* Because a global state change doesn't actually occur here, a read
* lock is sufficient.
@ -1382,10 +1386,6 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr *nam,
if (oinpp != NULL)
*oinpp = NULL;
if (nam->sa_len != sizeof (*sin))
return (EINVAL);
if (sin->sin_family != AF_INET)
return (EAFNOSUPPORT);
if (sin->sin_port == 0)
return (EADDRNOTAVAIL);
laddr.s_addr = *laddrp;

View file

@ -327,6 +327,22 @@ div_output(struct socket *so, struct mbuf *m, struct sockaddr_in *sin,
struct ipfw_rule_ref *dt;
int error, family;
if (control) {
m_freem(control); /* XXX */
control = NULL;
}
if (sin != NULL) {
if (sin->sin_family != AF_INET) {
m_freem(m);
return (EAFNOSUPPORT);
}
if (sin->sin_len != sizeof(*sin)) {
m_freem(m);
return (EINVAL);
}
}
/*
* An mbuf may hasn't come from userland, but we pretend
* that it has.
@ -335,9 +351,6 @@ div_output(struct socket *so, struct mbuf *m, struct sockaddr_in *sin,
m->m_nextpkt = NULL;
M_SETFIB(m, so->so_fibnum);
if (control)
m_freem(control); /* XXX */
mtag = m_tag_locate(m, MTAG_IPFW_RULE, 0, NULL);
if (mtag == NULL) {
/* this should be normal */
@ -628,6 +641,8 @@ div_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
*/
if (nam->sa_family != AF_INET)
return EAFNOSUPPORT;
if (nam->sa_len != sizeof(struct sockaddr_in))
return EINVAL;
((struct sockaddr_in *)nam)->sin_addr.s_addr = INADDR_ANY;
INP_INFO_WLOCK(&V_divcbinfo);
INP_WLOCK(inp);

View file

@ -996,6 +996,8 @@ rip_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
struct inpcb *inp;
int error;
if (nam->sa_family != AF_INET)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(*addr))
return (EINVAL);
@ -1070,6 +1072,7 @@ rip_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
{
struct inpcb *inp;
u_long dst;
int error;
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("rip_send: inp == NULL"));
@ -1084,9 +1087,16 @@ rip_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
}
dst = inp->inp_faddr.s_addr; /* Unlocked read. */
} else {
if (nam == NULL) {
error = 0;
if (nam == NULL)
error = ENOTCONN;
else if (nam->sa_family != AF_INET)
error = EAFNOSUPPORT;
else if (nam->sa_len != sizeof(struct sockaddr_in))
error = EINVAL;
if (error != 0) {
m_freem(m);
return (ENOTCONN);
return (error);
}
dst = ((struct sockaddr_in *)nam)->sin_addr.s_addr;
}

View file

@ -598,9 +598,20 @@ sctp_sendm(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
((inp->sctp_flags & SCTP_PCB_FLAGS_CONNECTED) ||
(inp->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE))) {
goto connected_type;
} else if (addr == NULL) {
}
error = 0;
if (addr == NULL) {
SCTP_LTRACE_ERR_RET_PKT(m, inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, EDESTADDRREQ);
error = EDESTADDRREQ;
} else if (addr->sa_family != AF_INET) {
SCTP_LTRACE_ERR_RET_PKT(m, inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, EAFNOSUPPORT);
error = EAFNOSUPPORT;
} else if (addr->sa_len != sizeof(struct sockaddr_in)) {
SCTP_LTRACE_ERR_RET_PKT(m, inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, EINVAL);
error = EINVAL;
}
if (error != 0) {
sctp_m_freem(m);
if (control) {
sctp_m_freem(control);
@ -608,19 +619,6 @@ sctp_sendm(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
}
return (error);
}
#ifdef INET6
if (addr->sa_family != AF_INET) {
/* must be a v4 address! */
SCTP_LTRACE_ERR_RET_PKT(m, inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, EDESTADDRREQ);
sctp_m_freem(m);
if (control) {
sctp_m_freem(control);
control = NULL;
}
error = EDESTADDRREQ;
return (error);
}
#endif /* INET6 */
connected_type:
/* now what about control */
if (control) {

View file

@ -321,14 +321,16 @@ tcp_usr_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
struct sockaddr_in *sinp;
sinp = (struct sockaddr_in *)nam;
if (nam->sa_len != sizeof (*sinp))
if (nam->sa_family != AF_INET)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(*sinp))
return (EINVAL);
/*
* Must check for multicast addresses and disallow binding
* to them.
*/
if (sinp->sin_family == AF_INET &&
IN_MULTICAST(ntohl(sinp->sin_addr.s_addr)))
if (IN_MULTICAST(ntohl(sinp->sin_addr.s_addr)))
return (EAFNOSUPPORT);
TCPDEBUG0;
@ -364,14 +366,16 @@ tcp6_usr_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
u_char vflagsav;
sin6 = (struct sockaddr_in6 *)nam;
if (nam->sa_len != sizeof (*sin6))
if (nam->sa_family != AF_INET6)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(*sin6))
return (EINVAL);
/*
* Must check for multicast addresses and disallow binding
* to them.
*/
if (sin6->sin6_family == AF_INET6 &&
IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr))
if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr))
return (EAFNOSUPPORT);
TCPDEBUG0;
@ -542,16 +546,17 @@ tcp_usr_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
struct sockaddr_in *sinp;
sinp = (struct sockaddr_in *)nam;
if (nam->sa_family != AF_INET)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof (*sinp))
return (EINVAL);
/*
* Must disallow TCP ``connections'' to multicast addresses.
*/
if (sinp->sin_family == AF_INET
&& IN_MULTICAST(ntohl(sinp->sin_addr.s_addr)))
if (IN_MULTICAST(ntohl(sinp->sin_addr.s_addr)))
return (EAFNOSUPPORT);
if ((sinp->sin_family == AF_INET) &&
(ntohl(sinp->sin_addr.s_addr) == INADDR_BROADCAST))
if (ntohl(sinp->sin_addr.s_addr) == INADDR_BROADCAST)
return (EACCES);
if ((error = prison_remote_ip4(td->td_ucred, &sinp->sin_addr)) != 0)
return (error);
@ -606,13 +611,15 @@ tcp6_usr_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
TCPDEBUG0;
sin6 = (struct sockaddr_in6 *)nam;
if (nam->sa_family != AF_INET6)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof (*sin6))
return (EINVAL);
/*
* Must disallow TCP ``connections'' to multicast addresses.
*/
if (sin6->sin6_family == AF_INET6
&& IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr))
if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr))
return (EAFNOSUPPORT);
inp = sotoinpcb(so);

View file

@ -1626,6 +1626,12 @@ udp_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
pcbinfo = udp_get_inpcbinfo(so->so_proto->pr_protocol);
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("udp_bind: inp == NULL"));
if (nam->sa_family != AF_INET)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(struct sockaddr_in))
return (EINVAL);
INP_WLOCK(inp);
INP_HASH_WLOCK(pcbinfo);
error = in_pcbbind(inp, nam, td->td_ucred);
@ -1666,12 +1672,18 @@ udp_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
pcbinfo = udp_get_inpcbinfo(so->so_proto->pr_protocol);
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("udp_connect: inp == NULL"));
sin = (struct sockaddr_in *)nam;
if (sin->sin_family != AF_INET)
return (EAFNOSUPPORT);
if (sin->sin_len != sizeof(*sin))
return (EINVAL);
INP_WLOCK(inp);
if (inp->inp_faddr.s_addr != INADDR_ANY) {
INP_WUNLOCK(inp);
return (EISCONN);
}
sin = (struct sockaddr_in *)nam;
error = prison_remote_ip4(td->td_ucred, &sin->sin_addr);
if (error != 0) {
INP_WUNLOCK(inp);
@ -1741,9 +1753,23 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
struct mbuf *control, struct thread *td)
{
struct inpcb *inp;
int error;
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("udp_send: inp == NULL"));
if (addr != NULL) {
error = 0;
if (addr->sa_family != AF_INET)
error = EAFNOSUPPORT;
else if (addr->sa_len != sizeof(struct sockaddr_in))
error = EINVAL;
if (__predict_false(error != 0)) {
m_freem(control);
m_freem(m);
return (error);
}
}
return (udp_output(inp, m, addr, control, td, flags));
}
#endif /* INET */

View file

@ -146,13 +146,10 @@ in6_pcbbind(struct inpcb *inp, struct sockaddr *nam,
return (error);
} else {
sin6 = (struct sockaddr_in6 *)nam;
if (nam->sa_len != sizeof(*sin6))
return (EINVAL);
/*
* family check.
*/
if (nam->sa_family != AF_INET6)
return (EAFNOSUPPORT);
KASSERT(sin6->sin6_family == AF_INET6,
("%s: invalid address family for %p", __func__, sin6));
KASSERT(sin6->sin6_len == sizeof(*sin6),
("%s: invalid address length for %p", __func__, sin6));
if ((error = sa6_embedscope(sin6, V_ip6_use_defzone)) != 0)
return(error);
@ -345,10 +342,9 @@ in6_pcbbind(struct inpcb *inp, struct sockaddr *nam,
* have forced minor changes in every protocol).
*/
static int
in6_pcbladdr(struct inpcb *inp, struct sockaddr *nam,
in6_pcbladdr(struct inpcb *inp, struct sockaddr_in6 *sin6,
struct in6_addr *plocal_addr6)
{
struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)nam;
int error = 0;
int scope_ambiguous = 0;
struct in6_addr in6a;
@ -357,10 +353,6 @@ in6_pcbladdr(struct inpcb *inp, struct sockaddr *nam,
INP_WLOCK_ASSERT(inp);
INP_HASH_WLOCK_ASSERT(inp->inp_pcbinfo); /* XXXRW: why? */
if (nam->sa_len != sizeof (*sin6))
return (EINVAL);
if (sin6->sin6_family != AF_INET6)
return (EAFNOSUPPORT);
if (sin6->sin6_port == 0)
return (EADDRNOTAVAIL);
@ -421,6 +413,11 @@ in6_pcbconnect_mbuf(struct inpcb *inp, struct sockaddr *nam,
struct sockaddr_in6 laddr6;
int error;
KASSERT(sin6->sin6_family == AF_INET6,
("%s: invalid address family for %p", __func__, sin6));
KASSERT(sin6->sin6_len == sizeof(*sin6),
("%s: invalid address length for %p", __func__, sin6));
bzero(&laddr6, sizeof(laddr6));
laddr6.sin6_family = AF_INET6;
@ -442,7 +439,7 @@ in6_pcbconnect_mbuf(struct inpcb *inp, struct sockaddr *nam,
* Call inner routine, to assign local interface address.
* in6_pcbladdr() may automatically fill in sin6_scope_id.
*/
if ((error = in6_pcbladdr(inp, nam, &laddr6.sin6_addr)) != 0)
if ((error = in6_pcbladdr(inp, sin6, &laddr6.sin6_addr)) != 0)
return (error);
if (in6_pcblookup_hash_locked(pcbinfo, &sin6->sin6_addr,

View file

@ -760,6 +760,8 @@ rip6_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("rip6_bind: inp == NULL"));
if (nam->sa_family != AF_INET6)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(*addr))
return (EINVAL);
if ((error = prison_check_ip6(td->td_ucred, &addr->sin6_addr)) != 0)
@ -891,6 +893,10 @@ rip6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
m_freem(m);
return (ENOTCONN);
}
if (nam->sa_family != AF_INET6) {
m_freem(m);
return (EAFNOSUPPORT);
}
if (nam->sa_len != sizeof(struct sockaddr_in6)) {
m_freem(m);
return (EINVAL);

View file

@ -709,6 +709,27 @@ sctp6_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EDESTADDRREQ);
return (EDESTADDRREQ);
}
switch (addr->sa_family) {
#ifdef INET
case AF_INET:
if (addr->sa_len != sizeof(struct sockaddr_in)) {
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL);
return (EINVAL);
}
break;
#endif
#ifdef INET6
case AF_INET6:
if (addr->sa_len != sizeof(struct sockaddr_in6)) {
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL);
return (EINVAL);
}
break;
#endif
default:
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL);
return (EINVAL);
}
#ifdef INET
sin6 = (struct sockaddr_in6 *)addr;
if (SCTP_IPV6_V6ONLY(inp)) {

View file

@ -231,6 +231,14 @@ send_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
__func__, so, V_send_so));
sendsrc = (struct sockaddr_send *)nam;
if (sendsrc->send_family != AF_INET6) {
error = EAFNOSUPPORT;
goto err;
}
if (sendsrc->send_len != sizeof(*sendsrc)) {
error = EINVAL;
goto err;
}
ifp = ifnet_byindex_ref(sendsrc->send_ifidx);
if (ifp == NULL) {
error = ENETUNREACH;

View file

@ -1091,6 +1091,11 @@ udp6_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
inp = sotoinpcb(so);
KASSERT(inp != NULL, ("udp6_bind: inp == NULL"));
if (nam->sa_family != AF_INET6)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(struct sockaddr_in6))
return (EINVAL);
INP_WLOCK(inp);
INP_HASH_WLOCK(pcbinfo);
vflagsav = inp->inp_vflag;
@ -1176,9 +1181,14 @@ udp6_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
pcbinfo = udp_get_inpcbinfo(so->so_proto->pr_protocol);
inp = sotoinpcb(so);
sin6 = (struct sockaddr_in6 *)nam;
KASSERT(inp != NULL, ("udp6_connect: inp == NULL"));
sin6 = (struct sockaddr_in6 *)nam;
if (sin6->sin6_family != AF_INET6)
return (EAFNOSUPPORT);
if (sin6->sin6_len != sizeof(*sin6))
return (EINVAL);
/*
* XXXRW: Need to clarify locking of v4/v6 flags.
*/

View file

@ -322,7 +322,7 @@ key_attach(struct socket *so, int proto, struct thread *td)
static int
key_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
{
return EINVAL;
return EINVAL;
}
/*

View file

@ -519,9 +519,9 @@ sdp_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
struct sockaddr_in *sin;
sin = (struct sockaddr_in *)nam;
if (nam->sa_len != sizeof (*sin))
return (EINVAL);
if (sin->sin_family != AF_INET)
return (EAFNOSUPPORT);
if (nam->sa_len != sizeof(*sin))
return (EINVAL);
if (IN_MULTICAST(ntohl(sin->sin_addr.s_addr)))
return (EAFNOSUPPORT);
@ -617,10 +617,10 @@ sdp_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
struct sockaddr_in *sin;
sin = (struct sockaddr_in *)nam;
if (nam->sa_len != sizeof (*sin))
if (nam->sa_len != sizeof(*sin))
return (EINVAL);
if (sin->sin_family != AF_INET)
return (EINVAL);
return (EAFNOSUPPORT);
if (IN_MULTICAST(ntohl(sin->sin_addr.s_addr)))
return (EAFNOSUPPORT);
if ((error = prison_remote_ip4(td->td_ucred, &sin->sin_addr)) != 0)
@ -932,6 +932,21 @@ sdp_send(struct socket *so, int flags, struct mbuf *m,
int error;
int cnt;
if (nam != NULL) {
if (nam->sa_family != AF_INET) {
if (control)
m_freem(control);
m_freem(m);
return (EAFNOSUPPORT);
}
if (nam->sa_len != sizeof(struct sockaddr_in)) {
if (control)
m_freem(control);
m_freem(m);
return (EINVAL);
}
}
error = 0;
ssk = sdp_sk(so);
KASSERT(m->m_flags & M_PKTHDR,