[PATCH nf] netfilter: nfnetlink_osf: fix divide-by-zero in OSF_WSS_MODULO

[PATCH nf] netfilter: nfnetlink_osf: fix divide-by-zero in OSF_WSS_MODULO

[Xiang Mei]

The OSF_WSS_MODULO branch in nf_osf_match_one() performs:

  ctx->window % f->wss.val

without guarding against f->wss.val == 0.  A user with CAP_NET_ADMIN
can add an OSF fingerprint with wss.wc = OSF_WSS_MODULO and wss.val = 0
via nfnetlink.  When a matching TCP SYN packet arrives, the kernel
executes a division by zero and panics.

The OSF_WSS_PLAIN case already treats val == 0 as a wildcard (match
everything).  Apply the same semantics to OSF_WSS_MODULO: if val is 0,
any window value matches rather than dividing by zero.

Crash:
 Oops: divide error: 0000 [#1] SMP KASAN NOPTI
 RIP: 0010:nf_osf_match_one (net/netfilter/nfnetlink_osf.c:98)
 Call Trace:
 <IRQ>
  nf_osf_match (net/netfilter/nfnetlink_osf.c:220 (discriminator 6))
  xt_osf_match_packet (net/netfilter/xt_osf.c:32)
  ipt_do_table (net/ipv4/netfilter/ip_tables.c:348)
  nf_hook_slow (net/netfilter/core.c:622 (discriminator 1))
  ip_local_deliver (net/ipv4/ip_input.c:265)
  ip_rcv (include/linux/skbuff.h:1162)
  __netif_receive_skb_one_core (net/core/dev.c:6181)
  process_backlog (.include/linux/skbuff.h:2502 net/core/dev.c:6642)
  __napi_poll (net/core/dev.c:7710)
  net_rx_action (net/core/dev.c:7945)
  handle_softirqs (kernel/softirq.c:622)

Fixes: 31a9c29210e2 ("netfilter: nf_osf: add struct nf_osf_hdr_ctx")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
 net/netfilter/nfnetlink_osf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
index 45d9ad231..193436aa9 100644
--- a/net/netfilter/nfnetlink_osf.c
+++ b/net/netfilter/nfnetlink_osf.c
@@ -150,7 +150,7 @@ static bool nf_osf_match_one(const struct sk_buff *skb,
 				fmatch = FMATCH_OK;
 			break;
 		case OSF_WSS_MODULO:
-			if ((ctx->window % f->wss.val) == 0)
+			if (f->wss.val == 0 || (ctx->window % f->wss.val) == 0)
 				fmatch = FMATCH_OK;
 			break;
 		}
-- 
2.43.0

Re: [PATCH nf] netfilter: nfnetlink_osf: fix divide-by-zero in OSF_WSS_MODULO

[Fernando Fernandez Mancera]

On 4/10/26 10:48 PM, Xiang Mei wrote:
> The OSF_WSS_MODULO branch in nf_osf_match_one() performs:
> 
>    ctx->window % f->wss.val
> 
> without guarding against f->wss.val == 0.  A user with CAP_NET_ADMIN
> can add an OSF fingerprint with wss.wc = OSF_WSS_MODULO and wss.val = 0
> via nfnetlink.  When a matching TCP SYN packet arrives, the kernel
> executes a division by zero and panics.
> 
> The OSF_WSS_PLAIN case already treats val == 0 as a wildcard (match
> everything).  Apply the same semantics to OSF_WSS_MODULO: if val is 0,
> any window value matches rather than dividing by zero.
> 
> Crash:
>   Oops: divide error: 0000 [#1] SMP KASAN NOPTI
>   RIP: 0010:nf_osf_match_one (net/netfilter/nfnetlink_osf.c:98)
>   Call Trace:
>   <IRQ>
>    nf_osf_match (net/netfilter/nfnetlink_osf.c:220 (discriminator 6))
>    xt_osf_match_packet (net/netfilter/xt_osf.c:32)
>    ipt_do_table (net/ipv4/netfilter/ip_tables.c:348)
>    nf_hook_slow (net/netfilter/core.c:622 (discriminator 1))
>    ip_local_deliver (net/ipv4/ip_input.c:265)
>    ip_rcv (include/linux/skbuff.h:1162)
>    __netif_receive_skb_one_core (net/core/dev.c:6181)
>    process_backlog (.include/linux/skbuff.h:2502 net/core/dev.c:6642)
>    __napi_poll (net/core/dev.c:7710)
>    net_rx_action (net/core/dev.c:7945)
>    handle_softirqs (kernel/softirq.c:622)
> 
> Fixes: 31a9c29210e2 ("netfilter: nf_osf: add struct nf_osf_hdr_ctx")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>

LGTM, but I don't see this happening at all to be honest. Such 
fingerprint would be bogus anyway.

The fixes tag is not correct I think. This was introduced on 
11eeef41d5f6 ("netfilter: passive OS fingerprint xtables match") actually.

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

Re: [PATCH nf] netfilter: nfnetlink_osf: fix divide-by-zero in OSF_WSS_MODULO

[Florian Westphal]

Xiang Mei <xmei5@asu.edu> wrote:
> The OSF_WSS_MODULO branch in nf_osf_match_one() performs:
> 
>   ctx->window % f->wss.val
> 
> without guarding against f->wss.val == 0.  A user with CAP_NET_ADMIN
> can add an OSF fingerprint with wss.wc = OSF_WSS_MODULO and wss.val = 0
> via nfnetlink.  When a matching TCP SYN packet arrives, the kernel
> executes a division by zero and panics.
> 
> The OSF_WSS_PLAIN case already treats val == 0 as a wildcard (match
> everything).  Apply the same semantics to OSF_WSS_MODULO: if val is 0,
> any window value matches rather than dividing by zero.
> 
> Crash:
>  Oops: divide error: 0000 [#1] SMP KASAN NOPTI
>  RIP: 0010:nf_osf_match_one (net/netfilter/nfnetlink_osf.c:98)
>  Call Trace:
>  <IRQ>
>   nf_osf_match (net/netfilter/nfnetlink_osf.c:220 (discriminator 6))
>   xt_osf_match_packet (net/netfilter/xt_osf.c:32)
>   ipt_do_table (net/ipv4/netfilter/ip_tables.c:348)
>   nf_hook_slow (net/netfilter/core.c:622 (discriminator 1))
>   ip_local_deliver (net/ipv4/ip_input.c:265)
>   ip_rcv (include/linux/skbuff.h:1162)
>   __netif_receive_skb_one_core (net/core/dev.c:6181)
>   process_backlog (.include/linux/skbuff.h:2502 net/core/dev.c:6642)
>   __napi_poll (net/core/dev.c:7710)
>   net_rx_action (net/core/dev.c:7945)
>   handle_softirqs (kernel/softirq.c:622)
> 
> Fixes: 31a9c29210e2 ("netfilter: nf_osf: add struct nf_osf_hdr_ctx")

Hmm, why?  AFAICS the bug was there from start:

11eeef41d5f63 case OSF_WSS_MODULO:
11eeef41d5f63    if ((window % f->wss.val) == 0)
11eeef41d5f63        fmatch = FMATCH_OK;

So:
Fixes: 11eeef41d5f6 ("netfilter: passive OS fingerprint xtables match")

> diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
> index 45d9ad231..193436aa9 100644
> --- a/net/netfilter/nfnetlink_osf.c
> +++ b/net/netfilter/nfnetlink_osf.c
> @@ -150,7 +150,7 @@ static bool nf_osf_match_one(const struct sk_buff *skb,
>  				fmatch = FMATCH_OK;
>  			break;
>  		case OSF_WSS_MODULO:
> -			if ((ctx->window % f->wss.val) == 0)
> +			if (f->wss.val == 0 || (ctx->window % f->wss.val) == 0)

Could you send a v2 that rejects this from control plane instead?
Nobody is using a 0 value, else we'd have gotted such crash reports by
now.