netfilter: ebtables: compat: reject all padding in matches/watchers

commit e608f631f0ba5f1fc5ee2e260a3a35d13107cbfe upstream.

syzbot reported following splat:

BUG: KASAN: vmalloc-out-of-bounds in size_entry_mwt net/bridge/netfilter/ebtables.c:2063 [inline]
BUG: KASAN: vmalloc-out-of-bounds in compat_copy_entries+0x128b/0x1380 net/bridge/netfilter/ebtables.c:2155
Read of size 4 at addr ffffc900004461f4 by task syz-executor267/7937

CPU: 1 PID: 7937 Comm: syz-executor267 Not tainted 5.5.0-rc1-syzkaller #0
 size_entry_mwt net/bridge/netfilter/ebtables.c:2063 [inline]
 compat_copy_entries+0x128b/0x1380 net/bridge/netfilter/ebtables.c:2155
 compat_do_replace+0x344/0x720 net/bridge/netfilter/ebtables.c:2249
 compat_do_ebt_set_ctl+0x22f/0x27e net/bridge/netfilter/ebtables.c:2333
 [..]

Because padding isn't considered during computation of ->buf_user_offset,
"total" is decremented by fewer bytes than it should.

Therefore, the first part of

if (*total < sizeof(*entry) || entry->next_offset < sizeof(*entry))

will pass, -- it should not have.  This causes oob access:
entry->next_offset is past the vmalloced size.

Reject padding and check that computed user offset (sum of ebt_entry
structure plus all individual matches/watchers/targets) is same
value that userspace gave us as the offset of the next entry.

Reported-by: syzbot+f68108fed972453a0ad4@syzkaller.appspotmail.com
Fixes: 81e675c227ec ("netfilter: ebtables: add CONFIG_COMPAT support")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 7d249af..785e19a 100644 (file)
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1876,7 +1876,7 @@ static int ebt_buf_count(struct ebt_entries_buf_state *state, unsigned int sz)
 }
 
 static int ebt_buf_add(struct ebt_entries_buf_state *state,
-                      void *data, unsigned int sz)
+                      const void *data, unsigned int sz)
 {
        if (state->buf_kern_start == NULL)
                goto count_only;
@@ -1910,7 +1910,7 @@ enum compat_mwt {
        EBT_COMPAT_TARGET,
 };
 
-static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
+static int compat_mtw_from_user(const struct compat_ebt_entry_mwt *mwt,
                                enum compat_mwt compat_mwt,
                                struct ebt_entries_buf_state *state,
                                const unsigned char *base)
@@ -1988,22 +1988,23 @@ static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
 /* return size of all matches, watchers or target, including necessary
  * alignment and padding.
  */
-static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
+static int ebt_size_mwt(const struct compat_ebt_entry_mwt *match32,
                        unsigned int size_left, enum compat_mwt type,
                        struct ebt_entries_buf_state *state, const void *base)
 {
+       const char *buf = (const char *)match32;
        int growth = 0;
-       char *buf;
 
        if (size_left == 0)
                return 0;
 
-       buf = (char *) match32;
-
-       while (size_left >= sizeof(*match32)) {
+       do {
                struct ebt_entry_match *match_kern;
                int ret;
 
+               if (size_left < sizeof(*match32))
+                       return -EINVAL;
+
                match_kern = (struct ebt_entry_match *) state->buf_kern_start;
                if (match_kern) {
                        char *tmp;
@@ -2040,22 +2041,18 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
                if (match_kern)
                        match_kern->match_size = ret;
 
-               /* rule should have no remaining data after target */
-               if (type == EBT_COMPAT_TARGET && size_left)
-                       return -EINVAL;
-
                match32 = (struct compat_ebt_entry_mwt *) buf;
-       }
+       } while (size_left);
 
        return growth;
 }
 
 /* called for all ebt_entry structures. */
-static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
+static int size_entry_mwt(const struct ebt_entry *entry, const unsigned char *base,
                          unsigned int *total,
                          struct ebt_entries_buf_state *state)
 {
-       unsigned int i, j, startoff, new_offset = 0;
+       unsigned int i, j, startoff, next_expected_off, new_offset = 0;
        /* stores match/watchers/targets & offset of next struct ebt_entry: */
        unsigned int offsets[4];
        unsigned int *offsets_update = NULL;
@@ -2141,11 +2138,13 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
                        return ret;
        }
 
-       startoff = state->buf_user_offset - startoff;
+       next_expected_off = state->buf_user_offset - startoff;
+       if (next_expected_off != entry->next_offset)
+               return -EINVAL;
 
-       if (WARN_ON(*total < startoff))
+       if (*total < entry->next_offset)
                return -EINVAL;
-       *total -= startoff;
+       *total -= entry->next_offset;
        return 0;
 }