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

[Pablo Neira Ayuso <pablo@netfilter.org>]

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.