Skip to content

Commit

Permalink
netfilter: bridge: ebt_among: add more missing match size checks
Browse files Browse the repository at this point in the history
ebt_among is special, it has a dynamic match size and is exempt
from the central size checks.

commit c4585a2 ("bridge: ebt_among: add missing match size checks")
added validation for pool size, but missed fact that the macros
ebt_among_wh_src/dst can already return out-of-bound result because
they do not check value of wh_src/dst_ofs (an offset) vs. the size
of the match that userspace gave to us.

v2:
check that offset has correct alignment.
Paolo Abeni points out that we should also check that src/dst
wormhash arrays do not overlap, and src + length lines up with
start of dst (or vice versa).
v3: compact wormhash_sizes_valid() part

NB: Fixes tag is intentionally wrong, this bug exists from day
one when match was added for 2.6 kernel. Tag is there so stable
maintainers will notice this one too.

Tested with same rules from the earlier patch.

Fixes: c4585a2 ("bridge: ebt_among: add missing match size checks")
Reported-by: <syzbot+bdabab6f1983a03fc009@syzkaller.appspotmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Florian Westphal authored and Pablo Neira Ayuso committed Mar 11, 2018
1 parent b1d0a5d commit c8d70a7
Showing 1 changed file with 34 additions and 0 deletions.
34 changes: 34 additions & 0 deletions net/bridge/netfilter/ebt_among.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,28 @@ static bool poolsize_invalid(const struct ebt_mac_wormhash *w)
return w && w->poolsize >= (INT_MAX / sizeof(struct ebt_mac_wormhash_tuple));
}

static bool wormhash_offset_invalid(int off, unsigned int len)
{
if (off == 0) /* not present */
return false;

if (off < (int)sizeof(struct ebt_among_info) ||
off % __alignof__(struct ebt_mac_wormhash))
return true;

off += sizeof(struct ebt_mac_wormhash);

return off > len;
}

static bool wormhash_sizes_valid(const struct ebt_mac_wormhash *wh, int a, int b)
{
if (a == 0)
a = sizeof(struct ebt_among_info);

return ebt_mac_wormhash_size(wh) + a == b;
}

static int ebt_among_mt_check(const struct xt_mtchk_param *par)
{
const struct ebt_among_info *info = par->matchinfo;
Expand All @@ -189,6 +211,10 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)
if (expected_length > em->match_size)
return -EINVAL;

if (wormhash_offset_invalid(info->wh_dst_ofs, em->match_size) ||
wormhash_offset_invalid(info->wh_src_ofs, em->match_size))
return -EINVAL;

wh_dst = ebt_among_wh_dst(info);
if (poolsize_invalid(wh_dst))
return -EINVAL;
Expand All @@ -201,6 +227,14 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)
if (poolsize_invalid(wh_src))
return -EINVAL;

if (info->wh_src_ofs < info->wh_dst_ofs) {
if (!wormhash_sizes_valid(wh_src, info->wh_src_ofs, info->wh_dst_ofs))
return -EINVAL;
} else {
if (!wormhash_sizes_valid(wh_dst, info->wh_dst_ofs, info->wh_src_ofs))
return -EINVAL;
}

expected_length += ebt_mac_wormhash_size(wh_src);

if (em->match_size != EBT_ALIGN(expected_length)) {
Expand Down

0 comments on commit c8d70a7

Please sign in to comment.