Re: [PATCH nf] netfilter: bridge: ebt_among: add missing match size checks

[Pablo Neira Ayuso]

On Mon, Feb 19, 2018 at 03:01:45AM +0100, Florian Westphal wrote:
> ebt_among is special, it has a dynamic match size and is exempt
> from the central size checks.
> 
> Therefore it must check that the size of the match structure
> provided from userspace is sane by making sure em->match_size
> is at least the minimum size of the expected structure.
> 
> The module has such a check, but its only done after accessing
> a structure that might be out of bounds.
> 
> tested with: ebtables -A INPUT ... \
> --among-dst fe:fe:fe:fe:fe:fe
> --among-dst fe:fe:fe:fe:fe:fe --among-src 
> fe:fe:fe:fe:ff:f,fe:fe:fe:fe:fe:fb,fe:fe:fe:fe:fc:fd,fe:fe:fe:fe:fe:fd,fe:fe:fe:fe:fe:fe
> --among-src 
> fe:fe:fe:fe:ff:f,fe:fe:fe:fe:fe:fa,fe:fe:fe:fe:fe:fd,fe:fe:fe:fe:fe:fe,fe:fe:fe:fe:fe:fe

Applied, thanks Florian.

[PATCH nf] netfilter: bridge: ebt_among: add missing match size checks

[Florian Westphal]

ebt_among is special, it has a dynamic match size and is exempt
from the central size checks.

Therefore it must check that the size of the match structure
provided from userspace is sane by making sure em->match_size
is at least the minimum size of the expected structure.

The module has such a check, but its only done after accessing
a structure that might be out of bounds.

tested with: ebtables -A INPUT ... \
--among-dst fe:fe:fe:fe:fe:fe
--among-dst fe:fe:fe:fe:fe:fe --among-src 
fe:fe:fe:fe:ff:f,fe:fe:fe:fe:fe:fb,fe:fe:fe:fe:fc:fd,fe:fe:fe:fe:fe:fd,fe:fe:fe:fe:fe:fe
--among-src 
fe:fe:fe:fe:ff:f,fe:fe:fe:fe:fe:fa,fe:fe:fe:fe:fe:fd,fe:fe:fe:fe:fe:fe,fe:fe:fe:fe:fe:fe

Reported-by: 
Signed-off-by: Florian Westphal 
---
 net/bridge/netfilter/ebt_among.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/net/bridge/netfilter/ebt_among.c b/net/bridge/netfilter/ebt_among.c
index ce7152a12bd8..c5afb4232ecb 100644
--- a/net/bridge/netfilter/ebt_among.c
+++ b/net/bridge/netfilter/ebt_among.c
@@ -172,18 +172,35 @@ ebt_among_mt(const struct sk_buff *skb, struct 
xt_action_param *par)
return true;
 }
 
+static bool poolsize_invalid(const struct ebt_mac_wormhash *w)
+{
+   return w && w->poolsize >= (INT_MAX / sizeof(struct 
ebt_mac_wormhash_tuple));
+}
+
 static int ebt_among_mt_check(const struct xt_mtchk_param *par)
 {
const struct ebt_among_info *info = par->matchinfo;
const struct ebt_entry_match *em =
container_of(par->matchinfo, const struct ebt_entry_match, 
data);
-   int expected_length = sizeof(struct ebt_among_info);
+   unsigned int expected_length = sizeof(struct ebt_among_info);
const struct ebt_mac_wormhash *wh_dst, *wh_src;
int err;
 
+   if (expected_length > em->match_size)
+   return -EINVAL;
+
wh_dst = ebt_among_wh_dst(info);
-   wh_src = ebt_among_wh_src(info);
+   if (poolsize_invalid(wh_dst))
+   return -EINVAL;
+
expected_length += ebt_mac_wormhash_size(wh_dst);
+   if (expected_length > em->match_size)
+   return -EINVAL;
+
+   wh_src = ebt_among_wh_src(info);
+   if (poolsize_invalid(wh_src))
+   return -EINVAL;
+
expected_length += ebt_mac_wormhash_size(wh_src);
 
if (em->match_size != EBT_ALIGN(expected_length)) {
-- 
2.16.1