Commit b4ad86bf authored by David S. Miller's avatar David S. Miller

[XFRM] xfrm_user: Better validation of user templates.

Since we never checked the ->family value of templates
before, many applications simply leave it at zero.
Detect this and fix it up to be the pol->family value.

Also, do not clobber xp->family while reading in templates,
that is not necessary.
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2b5f6dcc
...@@ -858,7 +858,6 @@ static void copy_templates(struct xfrm_policy *xp, struct xfrm_user_tmpl *ut, ...@@ -858,7 +858,6 @@ static void copy_templates(struct xfrm_policy *xp, struct xfrm_user_tmpl *ut,
int i; int i;
xp->xfrm_nr = nr; xp->xfrm_nr = nr;
xp->family = ut->family;
for (i = 0; i < nr; i++, ut++) { for (i = 0; i < nr; i++, ut++) {
struct xfrm_tmpl *t = &xp->xfrm_vec[i]; struct xfrm_tmpl *t = &xp->xfrm_vec[i];
...@@ -876,19 +875,53 @@ static void copy_templates(struct xfrm_policy *xp, struct xfrm_user_tmpl *ut, ...@@ -876,19 +875,53 @@ static void copy_templates(struct xfrm_policy *xp, struct xfrm_user_tmpl *ut,
} }
} }
static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family)
{
int i;
if (nr > XFRM_MAX_DEPTH)
return -EINVAL;
for (i = 0; i < nr; i++) {
/* We never validated the ut->family value, so many
* applications simply leave it at zero. The check was
* never made and ut->family was ignored because all
* templates could be assumed to have the same family as
* the policy itself. Now that we will have ipv4-in-ipv6
* and ipv6-in-ipv4 tunnels, this is no longer true.
*/
if (!ut[i].family)
ut[i].family = family;
switch (ut[i].family) {
case AF_INET:
break;
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
break;
#endif
default:
return -EINVAL;
};
}
return 0;
}
static int copy_from_user_tmpl(struct xfrm_policy *pol, struct rtattr **xfrma) static int copy_from_user_tmpl(struct xfrm_policy *pol, struct rtattr **xfrma)
{ {
struct rtattr *rt = xfrma[XFRMA_TMPL-1]; struct rtattr *rt = xfrma[XFRMA_TMPL-1];
struct xfrm_user_tmpl *utmpl;
int nr;
if (!rt) { if (!rt) {
pol->xfrm_nr = 0; pol->xfrm_nr = 0;
} else { } else {
nr = (rt->rta_len - sizeof(*rt)) / sizeof(*utmpl); struct xfrm_user_tmpl *utmpl = RTA_DATA(rt);
int nr = (rt->rta_len - sizeof(*rt)) / sizeof(*utmpl);
int err;
if (nr > XFRM_MAX_DEPTH) err = validate_tmpl(nr, utmpl, pol->family);
return -EINVAL; if (err)
return err;
copy_templates(pol, RTA_DATA(rt), nr); copy_templates(pol, RTA_DATA(rt), nr);
} }
...@@ -1530,7 +1563,8 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh, void **xf ...@@ -1530,7 +1563,8 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh, void **xf
} }
/* build an XP */ /* build an XP */
xp = xfrm_policy_construct(&ua->policy, (struct rtattr **) xfrma, &err); if (!xp) { xp = xfrm_policy_construct(&ua->policy, (struct rtattr **) xfrma, &err);
if (!xp) {
kfree(x); kfree(x);
return err; return err;
} }
...@@ -1979,7 +2013,7 @@ static struct xfrm_policy *xfrm_compile_policy(struct sock *sk, int opt, ...@@ -1979,7 +2013,7 @@ static struct xfrm_policy *xfrm_compile_policy(struct sock *sk, int opt,
return NULL; return NULL;
nr = ((len - sizeof(*p)) / sizeof(*ut)); nr = ((len - sizeof(*p)) / sizeof(*ut));
if (nr > XFRM_MAX_DEPTH) if (validate_tmpl(nr, ut, p->sel.family))
return NULL; return NULL;
if (p->dir > XFRM_POLICY_OUT) if (p->dir > XFRM_POLICY_OUT)
......
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