| Message ID | 20200813074611.281558-1-fw@strlen.de |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | [nf] netfilter/ebtables: reject bogus getopt len value | expand |
Looks good, sorry:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Thu, 13 Aug 2020 09:46:11 +0200 Florian Westphal wrote:
> Fixes: fc66de8e16e ("netfilter/ebtables: clean up compat {get, set}sockopt handling")
Fixes tag: Fixes: fc66de8e16e ("netfilter/ebtables: clean up compat {get, set}sockopt handling")
Has these problem(s):
- SHA1 should be at least 12 digits long
Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11
or later) just making sure it is not set (or set to "auto").
On Thu, Aug 13, 2020 at 09:46:11AM +0200, Florian Westphal wrote: > syzkaller reports splat: > ------------[ cut here ]------------ > Buffer overflow detected (80 < 137)! > Call Trace: > do_ebt_get_ctl+0x2b4/0x790 net/bridge/netfilter/ebtables.c:2317 > nf_getsockopt+0x72/0xd0 net/netfilter/nf_sockopt.c:116 > ip_getsockopt net/ipv4/ip_sockglue.c:1778 [inline] > > caused by a copy-to-user with a too-large "*len" value. > This adds a argument check on *len just like in the non-compat version > of the handler. > > Before the "Fixes" commit, the reproducer fails with -EINVAL as > expected: > 1. core calls the "compat" getsockopt version > 2. compat getsockopt version detects the *len value is possibly > in 64-bit layout (*len != compat_len) > 3. compat getsockopt version delegates everything to native getsockopt > version > 4. native getsockopt rejects invalid *len > > -> compat handler only sees len == sizeof(compat_struct) for GET_ENTRIES. > > After the refactor, event sequence is: > 1. getsockopt calls "compat" version (len != native_len) > 2. compat version attempts to copy *len bytes, where *len is random > value from userspace Applied, thanks.
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 1641f414d1ba..ebe33b60efd6 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -2238,6 +2238,10 @@ static int compat_do_ebt_get_ctl(struct sock *sk, int cmd, struct ebt_table *t; struct net *net = sock_net(sk); + if ((cmd == EBT_SO_GET_INFO || cmd == EBT_SO_GET_INIT_INFO) && + *len != sizeof(struct compat_ebt_replace)) + return -EINVAL; + if (copy_from_user(&tmp, user, sizeof(tmp))) return -EFAULT;
syzkaller reports splat: ------------[ cut here ]------------ Buffer overflow detected (80 < 137)! Call Trace: do_ebt_get_ctl+0x2b4/0x790 net/bridge/netfilter/ebtables.c:2317 nf_getsockopt+0x72/0xd0 net/netfilter/nf_sockopt.c:116 ip_getsockopt net/ipv4/ip_sockglue.c:1778 [inline] caused by a copy-to-user with a too-large "*len" value. This adds a argument check on *len just like in the non-compat version of the handler. Before the "Fixes" commit, the reproducer fails with -EINVAL as expected: 1. core calls the "compat" getsockopt version 2. compat getsockopt version detects the *len value is possibly in 64-bit layout (*len != compat_len) 3. compat getsockopt version delegates everything to native getsockopt version 4. native getsockopt rejects invalid *len -> compat handler only sees len == sizeof(compat_struct) for GET_ENTRIES. After the refactor, event sequence is: 1. getsockopt calls "compat" version (len != native_len) 2. compat version attempts to copy *len bytes, where *len is random value from userspace Fixes: fc66de8e16e ("netfilter/ebtables: clean up compat {get, set}sockopt handling") Reported-by: syzbot+5accb5c62faa1d346480@syzkaller.appspotmail.com Signed-off-by: Florian Westphal <fw@strlen.de> --- net/bridge/netfilter/ebtables.c | 4 ++++ 1 file changed, 4 insertions(+)