Commit e608f631 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso

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

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: 81e675c2 ("netfilter: ebtables: add CONFIG_COMPAT support")
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent d05d5db8
...@@ -1867,7 +1867,7 @@ static int ebt_buf_count(struct ebt_entries_buf_state *state, unsigned int sz) ...@@ -1867,7 +1867,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, 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) if (state->buf_kern_start == NULL)
goto count_only; goto count_only;
...@@ -1901,7 +1901,7 @@ enum compat_mwt { ...@@ -1901,7 +1901,7 @@ enum compat_mwt {
EBT_COMPAT_TARGET, 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, enum compat_mwt compat_mwt,
struct ebt_entries_buf_state *state, struct ebt_entries_buf_state *state,
const unsigned char *base) const unsigned char *base)
...@@ -1979,22 +1979,23 @@ static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt, ...@@ -1979,22 +1979,23 @@ static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
/* return size of all matches, watchers or target, including necessary /* return size of all matches, watchers or target, including necessary
* alignment and padding. * 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, unsigned int size_left, enum compat_mwt type,
struct ebt_entries_buf_state *state, const void *base) struct ebt_entries_buf_state *state, const void *base)
{ {
const char *buf = (const char *)match32;
int growth = 0; int growth = 0;
char *buf;
if (size_left == 0) if (size_left == 0)
return 0; return 0;
buf = (char *) match32; do {
while (size_left >= sizeof(*match32)) {
struct ebt_entry_match *match_kern; struct ebt_entry_match *match_kern;
int ret; int ret;
if (size_left < sizeof(*match32))
return -EINVAL;
match_kern = (struct ebt_entry_match *) state->buf_kern_start; match_kern = (struct ebt_entry_match *) state->buf_kern_start;
if (match_kern) { if (match_kern) {
char *tmp; char *tmp;
...@@ -2031,22 +2032,18 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32, ...@@ -2031,22 +2032,18 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
if (match_kern) if (match_kern)
match_kern->match_size = ret; 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; match32 = (struct compat_ebt_entry_mwt *) buf;
} } while (size_left);
return growth; return growth;
} }
/* called for all ebt_entry structures. */ /* 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, unsigned int *total,
struct ebt_entries_buf_state *state) 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: */ /* stores match/watchers/targets & offset of next struct ebt_entry: */
unsigned int offsets[4]; unsigned int offsets[4];
unsigned int *offsets_update = NULL; unsigned int *offsets_update = NULL;
...@@ -2132,11 +2129,13 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base, ...@@ -2132,11 +2129,13 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
return ret; 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; return -EINVAL;
*total -= startoff; *total -= entry->next_offset;
return 0; return 0;
} }
......
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