diff mbox series

[v2,nf] netfilter: ebtables: reject non-bridge targets

Message ID 20180606101456.3301-1-fw@strlen.de
State Accepted
Delegated to: Pablo Neira
Headers show
Series [v2,nf] netfilter: ebtables: reject non-bridge targets | expand

Commit Message

Florian Westphal June 6, 2018, 10:14 a.m. UTC
the ebtables evaluation loop expects targets to return
positive values (jumps), or negative values (absolute verdicts).

This is completely different from what xtables does.
In xtables, targets are expected to return the standard netfilter
verdicts, i.e. NF_DROP, NF_ACCEPT, etc.

ebtables will consider these as jumps.

Therefore reject any target found due to unspec fallback.
v2: also reject watchers.  ebtables ignores their return value, so
a taret that assumes skb ownership (and returns NF_STOLEN) causes
use-after-free.

The only watchers in the 'ebtables' front-end are log and nflog;
both have AF_BRIDGE specific wrappers on kernel side.

Reported-by: syzbot+2b43f681169a2a0d306a@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/netfilter/ebtables.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Pablo Neira Ayuso June 6, 2018, 1:03 p.m. UTC | #1
On Wed, Jun 06, 2018 at 12:14:56PM +0200, Florian Westphal wrote:
> the ebtables evaluation loop expects targets to return
> positive values (jumps), or negative values (absolute verdicts).
> 
> This is completely different from what xtables does.
> In xtables, targets are expected to return the standard netfilter
> verdicts, i.e. NF_DROP, NF_ACCEPT, etc.
> 
> ebtables will consider these as jumps.
> 
> Therefore reject any target found due to unspec fallback.
> v2: also reject watchers.  ebtables ignores their return value, so
> a taret that assumes skb ownership (and returns NF_STOLEN) causes
> use-after-free.
> 
> The only watchers in the 'ebtables' front-end are log and nflog;
> both have AF_BRIDGE specific wrappers on kernel side.

Applied, thanks Florian.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 6ba639f6c51d..62a303b68952 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -396,6 +396,12 @@  ebt_check_watcher(struct ebt_entry_watcher *w, struct xt_tgchk_param *par,
 	watcher = xt_request_find_target(NFPROTO_BRIDGE, w->u.name, 0);
 	if (IS_ERR(watcher))
 		return PTR_ERR(watcher);
+
+	if (watcher->family != NFPROTO_BRIDGE) {
+		module_put(watcher->me);
+		return -ENOENT;
+	}
+
 	w->u.watcher = watcher;
 
 	par->target   = watcher;
@@ -715,6 +721,13 @@  ebt_check_entry(struct ebt_entry *e, struct net *net,
 		goto cleanup_watchers;
 	}
 
+	/* Reject UNSPEC, xtables verdicts/return values are incompatible */
+	if (target->family != NFPROTO_BRIDGE) {
+		module_put(target->me);
+		ret = -ENOENT;
+		goto cleanup_watchers;
+	}
+
 	t->u.target = target;
 	if (t->u.target == &ebt_standard_target) {
 		if (gap < sizeof(struct ebt_standard_target)) {