mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] netfilter: xt_dccp: reject short option lengths
       [not found] <179251068646.3135.16276902372052104830@kernel.org>
@ 2026-11-05  8:17 ` shaos
  2026-11-05 11:01   ` Pablo Neira Ayuso
  2026-11-09  8:33   ` netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: shaos @ 2026-11-05  8:17 UTC (permalink / raw)
  To: pablo
  Cc: Sung Byeongchan, Florian Westphal, Phil Sutter, David S . Miller,
	Jakub Kicinski, Paolo Abeni, Eric Dumazet, Simon Horman,
	coreteam, netfilter-devel, netdev, linux-kernel

Hello,

The 20 October review of the reserved-option fix pointed out that
`dccp_find_option()` can mistake a short TLV length for the next option
type. The merged fix did not address that case. I reproduced it on
top of commit 2edda78aa4eb (the reserved-option fix).

With INPUT policy DROP and an ACCEPT rule for `--dccp-option 1`, an IPv4
DCCP packet containing options `{32, 1}` is accepted. Option 32 is a
multi-byte option, and length 1 is invalid. The walker advances by one
byte and then treats that length byte as Mandatory option 1. The rule
therefore accepts a malformed packet that does not contain option 1.
This affects firewalls that use DCCP option matches as an allow-list.
The match requires `CONFIG_NETFILTER_XT_MATCH_DCCP`; a remote sender
needs reachability to a firewall that uses an option-based DCCP rule.

The reproducer uses raw sockets and a local IPv4 DCCP packet. The QEMU
runs show the false match on the original kernel and after the merged
reserved-option fix. With this patch, the malformed packet is dropped.
A packet with a real Mandatory option 1 is accepted; without option 1,
it is dropped.

The earlier review is here:
https://lists.openwall.net/netdev/2026/10/20/477

Tested source revisions:
- Vulnerable: c03c1a14507dfaf8b856c2608f9ac216021fc222 (Linux 7.3)
- Vulnerable tree: 30fd2ef82912c26cf3a58ad0ba6afc375fa851ac
- Prior fix control: 2edda78aa4eb04cd8f112a81897510ee0cdd7099
- Patch control: 2edda78aa4eb04cd8f112a81897510ee0cdd7099 plus this
  patch

Git tree at patch base: 8644625317c9d6e32244509e29387fb0c80af0f4
Compiler: gcc 11.4.0, GNU ld 2.38
QEMU: 6.2.0, x86-64 TCG, 1024 MiB, 2 CPUs

Console output:
https://gist.githubusercontent.com/shaos26/a39f036057ddd1c1b78ce2da114b8f3a/raw/6a95ab42fd7f47c2e0113c55bc80a7e86002f222/console-output.txt
Kernel config:
https://gist.githubusercontent.com/shaos26/a39f036057ddd1c1b78ce2da114b8f3a/raw/f60c4bca22a42434cac67478e74d6f7c5b28442c/kernel-config.txt
PoC source:
https://gist.githubusercontent.com/shaos26/a39f036057ddd1c1b78ce2da114b8f3a/raw/3c45d0d340d804c236503cecf5ffe33f700245d0/poc.c

Check out commit 2edda78aa4eb. From that tree root, copy
`kernel-config.txt` to `.config` and run:

    make ARCH=x86_64 olddefconfig
    make ARCH=x86_64 -j"$(nproc)" bzImage

Compile the PoC with:

    gcc -static -O2 -Wall -Wextra -Werror -o poc poc.c

Boot the kernel in a disposable QEMU guest with BusyBox,
iptables-legacy, and `poc` available. Install this rule:

    iptables-legacy -F INPUT
    iptables-legacy -P INPUT DROP
    iptables-legacy -A INPUT -p 33 -m dccp --dccp-option 1 -j ACCEPT

Run `poc badlen1`, `poc present1`, and `poc absent1`. On the unpatched
kernel, the malformed packet is delivered, the packet with a real
Mandatory option is delivered, and the packet without option 1 is
dropped. With this patch, the malformed packet is dropped and the two
controls keep their expected results. The test needs CAP_NET_RAW to
synthesize and receive the local packet.

The option walker should reject a TLV length below two before advancing:

Fixes: 1d3de414eb20 ("[NETFILTER]: New iptables DCCP protocol header match")
Link: https://lists.openwall.net/netdev/2026/10/20/477
Signed-off-by: shaos <shawnq1@163.com>
---
 net/netfilter/xt_dccp.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/xt_dccp.c b/net/netfilter/xt_dccp.c
index 23a4e3e8df7d..e0d7f4374958 100644
--- a/net/netfilter/xt_dccp.c
+++ b/net/netfilter/xt_dccp.c
@@ -62,10 +62,15 @@
 			return true;
 		}
 
-		if (op[i] <= DCCPO_MAX_RESERVED || i == optlen - 1)
+		if (op[i] <= DCCPO_MAX_RESERVED || i == optlen - 1) {
 			i++;
-		else
-			i += op[i + 1] ? : 1;
+		} else {
+			if (op[i + 1] < 2) {
+				spin_unlock_bh(&dccp_buflock);
+				goto invalid;
+			}
+			i += op[i + 1];
+		}
 	}
 
 	spin_unlock_bh(&dccp_buflock);
 	return false;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] netfilter: xt_dccp: reject short option lengths
  2026-11-05  8:17 ` [PATCH net] netfilter: xt_dccp: reject short option lengths shaos
@ 2026-11-05 11:01   ` Pablo Neira Ayuso
  2026-11-09  8:33   ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2026-11-05 11:01 UTC (permalink / raw)
  To: shaos
  Cc: Sung Byeongchan, Florian Westphal, Phil Sutter, David S . Miller,
	Jakub Kicinski, Paolo Abeni, Eric Dumazet, Simon Horman,
	coreteam, netfilter-devel, netdev, linux-kernel

On Thu, Nov 05, 2026 at 08:17:35AM +0000, shaos wrote:
> Hello,
> 
> The 20 October review of the reserved-option fix pointed out that
> `dccp_find_option()` can mistake a short TLV length for the next option
> type. The merged fix did not address that case. I reproduced it on
> top of commit 2edda78aa4eb (the reserved-option fix).
> 
> With INPUT policy DROP and an ACCEPT rule for `--dccp-option 1`, an IPv4
> DCCP packet containing options `{32, 1}` is accepted. Option 32 is a
> multi-byte option, and length 1 is invalid. The walker advances by one
> byte and then treats that length byte as Mandatory option 1. The rule
> therefore accepts a malformed packet that does not contain option 1.
> This affects firewalls that use DCCP option matches as an allow-list.
> The match requires `CONFIG_NETFILTER_XT_MATCH_DCCP`; a remote sender
> needs reachability to a firewall that uses an option-based DCCP rule.
> 
> The reproducer uses raw sockets and a local IPv4 DCCP packet. The QEMU
> runs show the false match on the original kernel and after the merged
> reserved-option fix. With this patch, the malformed packet is dropped.
> A packet with a real Mandatory option 1 is accepted; without option 1,
> it is dropped.
> 
> The earlier review is here:
> https://lists.openwall.net/netdev/2026/10/20/477
> 
> Tested source revisions:
> - Vulnerable: c03c1a14507dfaf8b856c2608f9ac216021fc222 (Linux 7.3)
> - Vulnerable tree: 30fd2ef82912c26cf3a58ad0ba6afc375fa851ac
> - Prior fix control: 2edda78aa4eb04cd8f112a81897510ee0cdd7099
> - Patch control: 2edda78aa4eb04cd8f112a81897510ee0cdd7099 plus this
>   patch
> 
> Git tree at patch base: 8644625317c9d6e32244509e29387fb0c80af0f4
> Compiler: gcc 11.4.0, GNU ld 2.38
> QEMU: 6.2.0, x86-64 TCG, 1024 MiB, 2 CPUs
> 
> Console output:
> https://gist.githubusercontent.com/shaos26/a39f036057ddd1c1b78ce2da114b8f3a/raw/6a95ab42fd7f47c2e0113c55bc80a7e86002f222/console-output.txt
> Kernel config:
> https://gist.githubusercontent.com/shaos26/a39f036057ddd1c1b78ce2da114b8f3a/raw/f60c4bca22a42434cac67478e74d6f7c5b28442c/kernel-config.txt
> PoC source:
> https://gist.githubusercontent.com/shaos26/a39f036057ddd1c1b78ce2da114b8f3a/raw/3c45d0d340d804c236503cecf5ffe33f700245d0/poc.c
> 
> Check out commit 2edda78aa4eb. From that tree root, copy
> `kernel-config.txt` to `.config` and run:
> 
>     make ARCH=x86_64 olddefconfig
>     make ARCH=x86_64 -j"$(nproc)" bzImage
> 
> Compile the PoC with:
> 
>     gcc -static -O2 -Wall -Wextra -Werror -o poc poc.c
> 
> Boot the kernel in a disposable QEMU guest with BusyBox,
> iptables-legacy, and `poc` available. Install this rule:
> 
>     iptables-legacy -F INPUT
>     iptables-legacy -P INPUT DROP
>     iptables-legacy -A INPUT -p 33 -m dccp --dccp-option 1 -j ACCEPT
> 
> Run `poc badlen1`, `poc present1`, and `poc absent1`. On the unpatched
> kernel, the malformed packet is delivered, the packet with a real
> Mandatory option is delivered, and the packet without option 1 is
> dropped. With this patch, the malformed packet is dropped and the two
> controls keep their expected results. The test needs CAP_NET_RAW to
> synthesize and receive the local packet.
> 
> The option walker should reject a TLV length below two before advancing:
> 
> Fixes: 1d3de414eb20 ("[NETFILTER]: New iptables DCCP protocol header match")
> Link: https://lists.openwall.net/netdev/2026/10/20/477
> Signed-off-by: shaos <shawnq1@163.com>

This needs a real name.

> ---
>  net/netfilter/xt_dccp.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/xt_dccp.c b/net/netfilter/xt_dccp.c
> index 23a4e3e8df7d..e0d7f4374958 100644
> --- a/net/netfilter/xt_dccp.c
> +++ b/net/netfilter/xt_dccp.c
> @@ -62,10 +62,15 @@
>  			return true;
>  		}
>  
> -		if (op[i] <= DCCPO_MAX_RESERVED || i == optlen - 1)
> +		if (op[i] <= DCCPO_MAX_RESERVED || i == optlen - 1) {
>  			i++;
> -		else
> -			i += op[i + 1] ? : 1;
> +		} else {
> +			if (op[i + 1] < 2) {
> +				spin_unlock_bh(&dccp_buflock);
> +				goto invalid;
> +			}
> +			i += op[i + 1];
> +		}
>  	}
>  
>  	spin_unlock_bh(&dccp_buflock);
>  	return false;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] netfilter: xt_dccp: reject short option lengths
  2026-11-05  8:17 ` [PATCH net] netfilter: xt_dccp: reject short option lengths shaos
  2026-11-05 11:01   ` Pablo Neira Ayuso
@ 2026-11-09  8:33   ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-11-09  8:33 UTC (permalink / raw)
  To: shawnq1
  Cc: pablo, tjdqudcks0424, fw, phil, davem, kuba, pabeni, edumazet,
	horms, coreteam, netfilter-devel, netdev, linux-kernel

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch turns option lengths 0 and 1 into a hotdrop instead
  of a no-match, and the commit message does not say so.
- [Low] The commit body is an email cover letter pasted into the
  permanent changelog.
- [Low] Whether xt_dccp drops a malformed option TLV now depends on which
  malformed form it is and on option order.

--

netfilter: xt_dccp: reject short option lengths

This patch changes dccp_find_option() in net/netfilter/xt_dccp.c so that
a multi-byte DCCP option with a length byte of 0 or 1 is treated as
invalid. The walker previously advanced by one byte and could read the
length byte as the next option type. With this patch it sets hotdrop and
stops.

Link: https://lists.openwall.net/netdev/2026/10/20/477

> Hello,

[Severity: Low]
This isn't a bug, but the changelog reads like an email cover letter that
was pasted in as the commit body. It opens with "Hello,".

It also includes reproduction notes that would usually go below the ---
line: QEMU and gcc versions, gist URLs, tested SHAs, and build and boot
steps.

The technical explanation ends mid-sentence:

> The option walker should reject a TLV length below two before advancing:
>
> Fixes: 1d3de414eb20 ("[NETFILTER]: New iptables DCCP protocol header match")

Nothing follows the colon. The message also doesn't say what the code now
does with a length of 0 or 1, which is to hotdrop the packet.

> diff --git a/net/netfilter/xt_dccp.c b/net/netfilter/xt_dccp.c
> index 23a4e3e8df7de..e0d7f43749588 100644
> --- a/net/netfilter/xt_dccp.c
> +++ b/net/netfilter/xt_dccp.c
> @@ -62,10 +62,15 @@ dccp_find_option(u_int8_t option,
>  			return true;
>  		}
>  
> -		if (op[i] <= DCCPO_MAX_RESERVED || i == optlen - 1)
> +		if (op[i] <= DCCPO_MAX_RESERVED || i == optlen - 1) {
>  			i++;
> -		else
> -			i += op[i + 1] ? : 1;
> +		} else {
> +			if (op[i + 1] < 2) {
> +				spin_unlock_bh(&dccp_buflock);
> +				goto invalid;
> +			}

[Severity: Medium]
Is the change from a no-match to a hotdrop intended here?

The goto invalid path sets *hotdrop = true. dccp_mt() passes &par->hotdrop
down through match_option(), and ipt_do_table() then stops rule traversal
and drops the packet, whatever the target or chain policy:

net/ipv4/netfilter/ip_tables.c:ipt_do_table() {
    ...
	} while (!acpar.hotdrop);
    ...
	if (acpar.hotdrop)
		return NF_DROP;
    ...
}

Before this patch, a packet with options {32, 1} just failed the match and
kept going. With this patch it looks like the packet is also dropped in
setups such as these:

- an ACCEPT policy chain with a non-terminal --dccp-option N rule, for
  example LOG or MARK, or a rule in the mangle or raw table

- an inverted rule such as "! --dccp-option 5 -j ACCEPT", which used to
  match {32, 1} or {32, 0} because option 5 is absent

The commit message only describes the DROP policy allow-list case. In that
case hotdrop and no-match give the same result.

nft_exthdr_dccp_eval() handles the same malformed TLV as "option not
present" and does not drop the packet:

net/netfilter/nft_exthdr.c:nft_exthdr_dccp_eval() {
    ...
		if (len < 2)
			goto err;
    ...
err:
	*dest = 0;
}

xt_dccp already hotdrops when doff is shorter than the header, so hotdrop
may be the intended policy. Could the changelog say why hotdrop was chosen
over returning false? Could it also note that the iptables and nftables
DCCP option matches now give different results for these packets?

> +			i += op[i + 1];
> +		}
>  	}

[Severity: Low]
With the new hotdrop, dccp_find_option() now handles malformed TLVs in
different ways. The hotdrop only fires for a length of 0 or 1, and only if
the walker reaches that option before it finds the requested type.

Other malformed forms still end as a plain no-match:

- a length larger than the remaining option area, where
  i += op[i + 1] runs past optlen and the function returns false

- a multi-byte type (above DCCPO_MAX_RESERVED) in the last byte, which
  the i == optlen - 1 branch skips as if it were a single-byte option

Also, the op[i] == option check returns true before any length is checked.
So {32, 1} still matches --dccp-option 32, and {1, 32, 1} matches
--dccp-option 1 without the bad trailing TLV being examined.

Take an inverted rule like "! --dccp-option 5 -j ACCEPT". It would accept
{32, 0xff} and a trailing {..., 32}, but hotdrop {32, 1}, even though all
three are malformed. nft_exthdr_dccp_eval() treats every one of these as
"not present".

None of these cases brings back the false match this patch fixes. Once
lengths are at least 2, the walk stays on TLV boundaries.

Should these malformed forms all be handled the same way?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105081735.749379-1-shawnq1%40163.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-09  8:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <179251068646.3135.16276902372052104830@kernel.org>
2026-11-05  8:17 ` [PATCH net] netfilter: xt_dccp: reject short option lengths shaos
2026-11-05 11:01   ` Pablo Neira Ayuso
2026-11-09  8:33   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®