ports_match_v1() treats any non-zero pflags entry as the start of a port range and unconditionally consumes the next ports[] element as the range end. The checkentry path currently validates protocol, flags and count, but it does not reject a malformed rule whose final slot is marked as a range start. That leaves ports_match_v1() to step past the last valid ports[] element when such a rule is installed. Fix this by rejecting multiport v1 rules with a trailing range marker in checkentry. This keeps malformed rules out of the runtime matching path and preserves the existing exact-match and valid-range behavior. Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match") Reported-by: Yifan Wu Reported-by: Juefei Pu Co-developed-by: Yuan Tan Signed-off-by: Yuan Tan Suggested-by: Xin Liu Tested-by: Yuhang Zheng Signed-off-by: Ren Wei --- Changes in v2: - drop the selftest patch - send the fix publicly to netfilter-devel net/netfilter/xt_multiport.c | 50 +++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c index 44a00f5acde8..38aa5b90d38e 100644 --- a/net/netfilter/xt_multiport.c +++ b/net/netfilter/xt_multiport.c @@ -26,10 +26,10 @@ MODULE_ALIAS("ip6t_multiport"); /* Returns 1 if the port is matched by the test, 0 otherwise. */ static inline bool ports_match_v1(const struct xt_multiport_v1 *minfo, - u_int16_t src, u_int16_t dst) + u16 src, u16 dst) { unsigned int i; - u_int16_t s, e; + u16 s, e; for (i = 0; i < minfo->count; i++) { s = minfo->ports[i]; @@ -106,20 +106,36 @@ multiport_mt(const struct sk_buff *skb, struct xt_action_param *par) } static inline bool -check(u_int16_t proto, - u_int8_t ip_invflags, - u_int8_t match_flags, - u_int8_t count) +multiport_valid_ranges(const struct xt_multiport_v1 *multiinfo) +{ + unsigned int i; + + for (i = 0; i < multiinfo->count; i++) { + if (!multiinfo->pflags[i]) + continue; + + if (i == multiinfo->count - 1) + return false; + + i++; + } + + return true; +} + +static inline bool +check(u16 proto, u8 ip_invflags, const struct xt_multiport_v1 *multiinfo) { /* Must specify supported protocol, no unknown flags or bad count */ - return (proto == IPPROTO_TCP || proto == IPPROTO_UDP - || proto == IPPROTO_UDPLITE - || proto == IPPROTO_SCTP || proto == IPPROTO_DCCP) - && !(ip_invflags & XT_INV_PROTO) - && (match_flags == XT_MULTIPORT_SOURCE - || match_flags == XT_MULTIPORT_DESTINATION - || match_flags == XT_MULTIPORT_EITHER) - && count <= XT_MULTI_PORTS; + return (proto == IPPROTO_TCP || proto == IPPROTO_UDP || + proto == IPPROTO_UDPLITE || + proto == IPPROTO_SCTP || proto == IPPROTO_DCCP) && + !(ip_invflags & XT_INV_PROTO) && + (multiinfo->flags == XT_MULTIPORT_SOURCE || + multiinfo->flags == XT_MULTIPORT_DESTINATION || + multiinfo->flags == XT_MULTIPORT_EITHER) && + multiinfo->count <= XT_MULTI_PORTS && + multiport_valid_ranges(multiinfo); } static int multiport_mt_check(const struct xt_mtchk_param *par) @@ -127,8 +143,7 @@ static int multiport_mt_check(const struct xt_mtchk_param *par) const struct ipt_ip *ip = par->entryinfo; const struct xt_multiport_v1 *multiinfo = par->matchinfo; - return check(ip->proto, ip->invflags, multiinfo->flags, - multiinfo->count) ? 0 : -EINVAL; + return check(ip->proto, ip->invflags, multiinfo) ? 0 : -EINVAL; } static int multiport_mt6_check(const struct xt_mtchk_param *par) @@ -136,8 +151,7 @@ static int multiport_mt6_check(const struct xt_mtchk_param *par) const struct ip6t_ip6 *ip = par->entryinfo; const struct xt_multiport_v1 *multiinfo = par->matchinfo; - return check(ip->proto, ip->invflags, multiinfo->flags, - multiinfo->count) ? 0 : -EINVAL; + return check(ip->proto, ip->invflags, multiinfo) ? 0 : -EINVAL; } static struct xt_match multiport_mt_reg[] __read_mostly = { -- 2.51.0