Commit 096f41d3 authored by Herbert Xu's avatar Herbert Xu Committed by Steffen Klassert

af_key: Fix sadb_x_ipsecrequest parsing

The parsing of sadb_x_ipsecrequest is broken in a number of ways.
First of all we're not verifying sadb_x_ipsecrequest_len.  This
is needed when the structure carries addresses at the end.  Worse
we don't even look at the length when we parse those optional
addresses.

The migration code had similar parsing code that's better but
it also has some deficiencies.  The length is overcounted first
of all as it includes the header itself.  It also fails to check
the length before dereferencing the sa_family field.

This patch fixes those problems in parse_sockaddr_pair and then
uses it in parse_ipsecrequest.
Reported-by: default avatarAndrey Konovalov <andreyknvl@google.com>
Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarSteffen Klassert <steffen.klassert@secunet.com>
parent 89e357d8
...@@ -66,6 +66,10 @@ struct pfkey_sock { ...@@ -66,6 +66,10 @@ struct pfkey_sock {
struct mutex dump_lock; struct mutex dump_lock;
}; };
static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len,
xfrm_address_t *saddr, xfrm_address_t *daddr,
u16 *family);
static inline struct pfkey_sock *pfkey_sk(struct sock *sk) static inline struct pfkey_sock *pfkey_sk(struct sock *sk)
{ {
return (struct pfkey_sock *)sk; return (struct pfkey_sock *)sk;
...@@ -1939,19 +1943,14 @@ parse_ipsecrequest(struct xfrm_policy *xp, struct sadb_x_ipsecrequest *rq) ...@@ -1939,19 +1943,14 @@ parse_ipsecrequest(struct xfrm_policy *xp, struct sadb_x_ipsecrequest *rq)
/* addresses present only in tunnel mode */ /* addresses present only in tunnel mode */
if (t->mode == XFRM_MODE_TUNNEL) { if (t->mode == XFRM_MODE_TUNNEL) {
u8 *sa = (u8 *) (rq + 1); int err;
int family, socklen;
family = pfkey_sockaddr_extract((struct sockaddr *)sa,
&t->saddr);
if (!family)
return -EINVAL;
socklen = pfkey_sockaddr_len(family); err = parse_sockaddr_pair(
if (pfkey_sockaddr_extract((struct sockaddr *)(sa + socklen), (struct sockaddr *)(rq + 1),
&t->id.daddr) != family) rq->sadb_x_ipsecrequest_len - sizeof(*rq),
return -EINVAL; &t->saddr, &t->id.daddr, &t->encap_family);
t->encap_family = family; if (err)
return err;
} else } else
t->encap_family = xp->family; t->encap_family = xp->family;
...@@ -1971,7 +1970,11 @@ parse_ipsecrequests(struct xfrm_policy *xp, struct sadb_x_policy *pol) ...@@ -1971,7 +1970,11 @@ parse_ipsecrequests(struct xfrm_policy *xp, struct sadb_x_policy *pol)
if (pol->sadb_x_policy_len * 8 < sizeof(struct sadb_x_policy)) if (pol->sadb_x_policy_len * 8 < sizeof(struct sadb_x_policy))
return -EINVAL; return -EINVAL;
while (len >= sizeof(struct sadb_x_ipsecrequest)) { while (len >= sizeof(*rq)) {
if (len < rq->sadb_x_ipsecrequest_len ||
rq->sadb_x_ipsecrequest_len < sizeof(*rq))
return -EINVAL;
if ((err = parse_ipsecrequest(xp, rq)) < 0) if ((err = parse_ipsecrequest(xp, rq)) < 0)
return err; return err;
len -= rq->sadb_x_ipsecrequest_len; len -= rq->sadb_x_ipsecrequest_len;
...@@ -2434,7 +2437,6 @@ static int key_pol_get_resp(struct sock *sk, struct xfrm_policy *xp, const struc ...@@ -2434,7 +2437,6 @@ static int key_pol_get_resp(struct sock *sk, struct xfrm_policy *xp, const struc
return err; return err;
} }
#ifdef CONFIG_NET_KEY_MIGRATE
static int pfkey_sockaddr_pair_size(sa_family_t family) static int pfkey_sockaddr_pair_size(sa_family_t family)
{ {
return PFKEY_ALIGN8(pfkey_sockaddr_len(family) * 2); return PFKEY_ALIGN8(pfkey_sockaddr_len(family) * 2);
...@@ -2446,7 +2448,7 @@ static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len, ...@@ -2446,7 +2448,7 @@ static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len,
{ {
int af, socklen; int af, socklen;
if (ext_len < pfkey_sockaddr_pair_size(sa->sa_family)) if (ext_len < 2 || ext_len < pfkey_sockaddr_pair_size(sa->sa_family))
return -EINVAL; return -EINVAL;
af = pfkey_sockaddr_extract(sa, saddr); af = pfkey_sockaddr_extract(sa, saddr);
...@@ -2462,6 +2464,7 @@ static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len, ...@@ -2462,6 +2464,7 @@ static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len,
return 0; return 0;
} }
#ifdef CONFIG_NET_KEY_MIGRATE
static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len, static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
struct xfrm_migrate *m) struct xfrm_migrate *m)
{ {
...@@ -2469,13 +2472,14 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len, ...@@ -2469,13 +2472,14 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
struct sadb_x_ipsecrequest *rq2; struct sadb_x_ipsecrequest *rq2;
int mode; int mode;
if (len <= sizeof(struct sadb_x_ipsecrequest) || if (len < sizeof(*rq1) ||
len < rq1->sadb_x_ipsecrequest_len) len < rq1->sadb_x_ipsecrequest_len ||
rq1->sadb_x_ipsecrequest_len < sizeof(*rq1))
return -EINVAL; return -EINVAL;
/* old endoints */ /* old endoints */
err = parse_sockaddr_pair((struct sockaddr *)(rq1 + 1), err = parse_sockaddr_pair((struct sockaddr *)(rq1 + 1),
rq1->sadb_x_ipsecrequest_len, rq1->sadb_x_ipsecrequest_len - sizeof(*rq1),
&m->old_saddr, &m->old_daddr, &m->old_saddr, &m->old_daddr,
&m->old_family); &m->old_family);
if (err) if (err)
...@@ -2484,13 +2488,14 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len, ...@@ -2484,13 +2488,14 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
rq2 = (struct sadb_x_ipsecrequest *)((u8 *)rq1 + rq1->sadb_x_ipsecrequest_len); rq2 = (struct sadb_x_ipsecrequest *)((u8 *)rq1 + rq1->sadb_x_ipsecrequest_len);
len -= rq1->sadb_x_ipsecrequest_len; len -= rq1->sadb_x_ipsecrequest_len;
if (len <= sizeof(struct sadb_x_ipsecrequest) || if (len <= sizeof(*rq2) ||
len < rq2->sadb_x_ipsecrequest_len) len < rq2->sadb_x_ipsecrequest_len ||
rq2->sadb_x_ipsecrequest_len < sizeof(*rq2))
return -EINVAL; return -EINVAL;
/* new endpoints */ /* new endpoints */
err = parse_sockaddr_pair((struct sockaddr *)(rq2 + 1), err = parse_sockaddr_pair((struct sockaddr *)(rq2 + 1),
rq2->sadb_x_ipsecrequest_len, rq2->sadb_x_ipsecrequest_len - sizeof(*rq2),
&m->new_saddr, &m->new_daddr, &m->new_saddr, &m->new_daddr,
&m->new_family); &m->new_family);
if (err) if (err)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment