| Message ID | 1458666173-24318-4-git-send-email-fw@strlen.de |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
On Tue, Mar 22, 2016 at 06:02:51PM +0100, Florian Westphal wrote: > We have targets and standard targets -- the latter carries a verdict, so we > must check the standard size as well here -- later functions access t->verdict > which otherwise can point after the blob end. Applied to nf, thanks. -- 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
Florian Westphal <fw@strlen.de> wrote: > + if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 && > + target_offset + sizeof(struct xt_standard_target) > next_offset) > + return -EINVAL; The last hunk broke CONFIG_COMPAT, it calls into check_entry() and casts compat_Xt_entry to Xt_entry, but for compat case xt_standard_target is larger than what a 32bit userspace sends us. For now I'd suggest to not pass this to stable, I'll rework the compat handlers next week to fix this properly. -- 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
On Fri, Mar 25, 2016 at 12:45:53PM +0100, Florian Westphal wrote: > Florian Westphal <fw@strlen.de> wrote: > > + if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 && > > + target_offset + sizeof(struct xt_standard_target) > next_offset) > > + return -EINVAL; > > The last hunk broke CONFIG_COMPAT, it calls into check_entry() and > casts compat_Xt_entry to Xt_entry, but for compat case > xt_standard_target is larger than what a 32bit userspace sends us. > > For now I'd suggest to not pass this to stable, I'll rework > the compat handlers next week to fix this properly. OK, I'll pull this out from the nf tree and rebase. -- 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 --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h index 80a305b..9e7153e 100644 --- a/include/linux/netfilter/x_tables.h +++ b/include/linux/netfilter/x_tables.h @@ -413,6 +413,9 @@ xt_get_per_cpu_counter(struct xt_counters *cnt, unsigned int cpu) struct nf_hook_ops *xt_hook_ops_alloc(const struct xt_table *, nf_hookfn *); +int xt_check_entry_target(const void *base, unsigned int target_offset, + unsigned int next_offset); + #ifdef CONFIG_COMPAT #include <net/compat.h> diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 51d4fe5..59a9610 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -476,19 +476,10 @@ next: static inline int check_entry(const struct arpt_entry *e) { - const struct xt_entry_target *t; - if (!arp_checkentry(&e->arp)) return -EINVAL; - if (e->target_offset + sizeof(struct xt_entry_target) > e->next_offset) - return -EINVAL; - - t = arpt_get_target_c(e); - if (e->target_offset + t->u.target_size > e->next_offset) - return -EINVAL; - - return 0; + return xt_check_entry_target(e, e->target_offset, e->next_offset); } static inline int check_target(struct arpt_entry *e, const char *name) diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index fb7694e..1acc6ee 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -571,20 +571,10 @@ static void cleanup_match(struct xt_entry_match *m, struct net *net) static int check_entry(const struct ipt_entry *e) { - const struct xt_entry_target *t; - if (!ip_checkentry(&e->ip)) return -EINVAL; - if (e->target_offset + sizeof(struct xt_entry_target) > - e->next_offset) - return -EINVAL; - - t = ipt_get_target_c(e); - if (e->target_offset + t->u.target_size > e->next_offset) - return -EINVAL; - - return 0; + return xt_check_entry_target(e, e->target_offset, e->next_offset); } static int diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index b248528f..29e2113 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -583,20 +583,10 @@ static void cleanup_match(struct xt_entry_match *m, struct net *net) static int check_entry(const struct ip6t_entry *e) { - const struct xt_entry_target *t; - if (!ip6_checkentry(&e->ipv6)) return -EINVAL; - if (e->target_offset + sizeof(struct xt_entry_target) > - e->next_offset) - return -EINVAL; - - t = ip6t_get_target_c(e); - if (e->target_offset + t->u.target_size > e->next_offset) - return -EINVAL; - - return 0; + return xt_check_entry_target(e, e->target_offset, e->next_offset); } static int check_match(struct xt_entry_match *m, struct xt_mtchk_param *par) diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 582c9cf..c10f9b3 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -541,6 +541,28 @@ int xt_compat_match_to_user(const struct xt_entry_match *m, EXPORT_SYMBOL_GPL(xt_compat_match_to_user); #endif /* CONFIG_COMPAT */ +/* check that arp/ip/ip6t_entry target_offset is sane */ +int xt_check_entry_target(const void *base, unsigned int target_offset, + unsigned int next_offset) +{ + const struct xt_entry_target *t; + const char *e = base; + + if (target_offset + sizeof(struct xt_entry_target) > next_offset) + return -EINVAL; + + t = (void *) (e + target_offset); + if (target_offset + t->u.target_size > next_offset) + return -EINVAL; + + if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 && + target_offset + sizeof(struct xt_standard_target) > next_offset) + return -EINVAL; + + return 0; +} +EXPORT_SYMBOL(xt_check_entry_target); + int xt_check_target(struct xt_tgchk_param *par, unsigned int size, u_int8_t proto, bool inv_proto) {
We have targets and standard targets -- the latter carries a verdict, so we must check the standard size as well here -- later functions access t->verdict which otherwise can point after the blob end. Spotted with UBSAN. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/linux/netfilter/x_tables.h | 3 +++ net/ipv4/netfilter/arp_tables.c | 11 +---------- net/ipv4/netfilter/ip_tables.c | 12 +----------- net/ipv6/netfilter/ip6_tables.c | 12 +----------- net/netfilter/x_tables.c | 22 ++++++++++++++++++++++ 5 files changed, 28 insertions(+), 32 deletions(-)