Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 35b61a14 authored by Florian Westphal's avatar Florian Westphal Committed by Greg Kroah-Hartman
Browse files

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: default avatar <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>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 19446871

net/bridge/netfilter/ebtables.c

+16 −17
Original line number Diff line number Diff line
@@ -1894,7 +1894,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;
@@ -1928,7 +1928,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)
@@ -2004,22 +2004,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;
@@ -2056,22 +2057,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;
@@ -2158,11 +2155,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;

}