[PATCH v3] netfilter: xt_multiport: validate range encoding in checkentry

[PATCH v3] netfilter: xt_multiport: validate range encoding in checkentry

[Ren Wei]

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 validate the range encoding itself. As a result, malformed
rules can mark the last slot as a range start or place two range starts
back to back, leaving ports_match_v1() to step past the last valid
ports[] element while interpreting the rule.

Reject malformed multiport v1 rules in checkentry by validating that
each range start has a following element and that the following element
is not itself marked as another range start.

Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match")
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Co-developed-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Yuan Tan <yuantan098@gmail.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Yuhang Zheng <z1652074432@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v2:
- drop the selftest patch
- send the fix publicly to netfilter-devel

Changes in v3:
- drop datatype cleanup from the fix
- keep the original check() interface unchanged
- validate malformed range encoding in checkentry

 net/netfilter/xt_multiport.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
index 44a00f5acde8..07a0f2a3fc75 100644
--- a/net/netfilter/xt_multiport.c
+++ b/net/netfilter/xt_multiport.c
@@ -105,6 +105,24 @@ multiport_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	return ports_match_v1(multiinfo, ntohs(pptr[0]), ntohs(pptr[1]));
 }
 
+static inline bool
+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 + 1 >= multiinfo->count || multiinfo->pflags[i + 1])
+			return false;
+
+		i++;
+	}
+
+	return true;
+}
+
 static inline bool
 check(u_int16_t proto,
       u_int8_t ip_invflags,
@@ -127,8 +145,10 @@ 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;
+	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
+		return -EINVAL;
+
+	return multiport_valid_ranges(multiinfo) ? 0 : -EINVAL;
 }
 
 static int multiport_mt6_check(const struct xt_mtchk_param *par)
@@ -136,8 +156,10 @@ 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;
+	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
+		return -EINVAL;
+
+	return multiport_valid_ranges(multiinfo) ? 0 : -EINVAL;
 }
 
 static struct xt_match multiport_mt_reg[] __read_mostly = {
-- 
2.51.0

Re: [PATCH v3] netfilter: xt_multiport: validate range encoding in checkentry

[Pablo Neira Ayuso]

On Fri, Apr 03, 2026 at 02:21:17AM +0800, Ren Wei wrote:
> 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 validate the range encoding itself. As a result, malformed
> rules can mark the last slot as a range start or place two range starts
> back to back, leaving ports_match_v1() to step past the last valid
> ports[] element while interpreting the rule.
> 
> Reject malformed multiport v1 rules in checkentry by validating that
> each range start has a following element and that the following element
> is not itself marked as another range start.
> 
> Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Yuhang Zheng <z1652074432@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> Changes in v2:
> - drop the selftest patch
> - send the fix publicly to netfilter-devel
> 
> Changes in v3:
> - drop datatype cleanup from the fix
> - keep the original check() interface unchanged
> - validate malformed range encoding in checkentry
> 
>  net/netfilter/xt_multiport.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
> index 44a00f5acde8..07a0f2a3fc75 100644
> --- a/net/netfilter/xt_multiport.c
> +++ b/net/netfilter/xt_multiport.c
> @@ -105,6 +105,24 @@ multiport_mt(const struct sk_buff *skb, struct xt_action_param *par)
>  	return ports_match_v1(multiinfo, ntohs(pptr[0]), ntohs(pptr[1]));
>  }
>  
> +static inline bool
> +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 + 1 >= multiinfo->count || multiinfo->pflags[i + 1])
> +			return false;
> +
> +		i++;
> +	}
> +
> +	return true;
> +}

I'd suggest:

static inline bool
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)
                        return false;
         
                if (multiinfo->pflags[i])
                        return false;
         
                if (multiinfo->ports[i - 1] > multiinfo->ports[i])
                        return false;
        }
 
        return true;
}

Then, this validate non-sense ports array too.

Re: [PATCH v3] netfilter: xt_multiport: validate range encoding in checkentry

[Pablo Neira Ayuso]

On Fri, Apr 03, 2026 at 01:31:41PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Apr 03, 2026 at 02:21:17AM +0800, Ren Wei wrote:
> > 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 validate the range encoding itself. As a result, malformed
> > rules can mark the last slot as a range start or place two range starts
> > back to back, leaving ports_match_v1() to step past the last valid
> > ports[] element while interpreting the rule.
> > 
> > Reject malformed multiport v1 rules in checkentry by validating that
> > each range start has a following element and that the following element
> > is not itself marked as another range start.
> > 
> > Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match")
> > Reported-by: Yifan Wu <yifanwucs@gmail.com>
> > Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> > Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> > Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> > Suggested-by: Xin Liu <bird@lzu.edu.cn>
> > Tested-by: Yuhang Zheng <z1652074432@gmail.com>
> > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> > ---
> > Changes in v2:
> > - drop the selftest patch
> > - send the fix publicly to netfilter-devel
> > 
> > Changes in v3:
> > - drop datatype cleanup from the fix
> > - keep the original check() interface unchanged
> > - validate malformed range encoding in checkentry
> > 
> >  net/netfilter/xt_multiport.c | 30 ++++++++++++++++++++++++++----
> >  1 file changed, 26 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
> > index 44a00f5acde8..07a0f2a3fc75 100644
> > --- a/net/netfilter/xt_multiport.c
> > +++ b/net/netfilter/xt_multiport.c
> > @@ -105,6 +105,24 @@ multiport_mt(const struct sk_buff *skb, struct xt_action_param *par)
> >  	return ports_match_v1(multiinfo, ntohs(pptr[0]), ntohs(pptr[1]));
> >  }
> >  
> > +static inline bool
> > +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 + 1 >= multiinfo->count || multiinfo->pflags[i + 1])
> > +			return false;
> > +
> > +		i++;
> > +	}
> > +
> > +	return true;
> > +}
> 
> I'd suggest:
> 
> static inline bool

Actually, inline is silly here, no inline here.

> 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)
>                         return false;
>          
>                 if (multiinfo->pflags[i])
>                         return false;
>          
>                 if (multiinfo->ports[i - 1] > multiinfo->ports[i])
>                         return false;
>         }
>  
>         return true;
> }
> 
> Then, this validate non-sense ports array too.

You can also mention in the patch description that this leads to
off-by-one read after the array that is reported via UBSAN.

Thanks.

[PATCH v4] netfilter: xt_multiport: validate range encoding in checkentry

[Ren Wei]

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 validate the range encoding itself. As a result, malformed
rules can mark the last slot as a range start or place two range starts
back to back, leaving ports_match_v1() to step past the last valid
ports[] element while interpreting the rule.

Reject malformed multiport v1 rules in checkentry by validating that
each range start has a following element and that the following element
is not itself marked as another range start.

Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match")
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Co-developed-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Yuan Tan <yuantan098@gmail.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Yuhang Zheng <z1652074432@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v2:
- drop the selftest patch
- send the fix publicly to netfilter-devel

Changes in v3:
- drop datatype cleanup from the fix
- keep the original check() interface unchanged
- validate malformed range encoding in checkentry

Changes in v4:
- follow Pablo's suggested validation form
- drop inline from multiport_valid_ranges
- reject reversed port ranges

 net/netfilter/xt_multiport.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
index 44a00f5acde8..a1691ff405d3 100644
--- a/net/netfilter/xt_multiport.c
+++ b/net/netfilter/xt_multiport.c
@@ -105,6 +105,28 @@ multiport_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	return ports_match_v1(multiinfo, ntohs(pptr[0]), ntohs(pptr[1]));
 }
 
+static bool
+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)
+			return false;
+
+		if (multiinfo->pflags[i])
+			return false;
+
+		if (multiinfo->ports[i - 1] > multiinfo->ports[i])
+			return false;
+	}
+
+	return true;
+}
+
 static inline bool
 check(u_int16_t proto,
       u_int8_t ip_invflags,
@@ -127,8 +149,10 @@ 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;
+	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
+		return -EINVAL;
+
+	return multiport_valid_ranges(multiinfo) ? 0 : -EINVAL;
 }
 
 static int multiport_mt6_check(const struct xt_mtchk_param *par)
@@ -136,8 +160,10 @@ 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;
+	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
+		return -EINVAL;
+
+	return multiport_valid_ranges(multiinfo) ? 0 : -EINVAL;
 }
 
 static struct xt_match multiport_mt_reg[] __read_mostly = {
-- 
2.51.0