NetBSD Problem Report #59511
From www@netbsd.org Sat Jul 5 10:38:30 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
client-signature RSA-PSS (2048 bits) client-digest SHA256)
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 9050D1A923E
for <gnats-bugs@gnats.NetBSD.org>; Sat, 5 Jul 2025 10:38:30 +0000 (UTC)
Message-Id: <20250705103829.5C9CD1A923F@mollari.NetBSD.org>
Date: Sat, 5 Jul 2025 10:38:29 +0000 (UTC)
From: emmankoko519@gmail.com
Reply-To: emmankoko519@gmail.com
To: gnats-bugs@NetBSD.org
Subject: some variable addresses not processed by firewall rules.
X-Send-Pr-Version: www-1.0
>Number: 59511
>Category: bin
>Synopsis: some variable addresses not processed by firewall rules.
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: joe
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat Jul 05 10:40:00 +0000 2025
>Last-Modified: Wed Aug 20 11:05:02 +0000 2025
>Originator: Emmanuel Nyarko
>Release: NetBSD 10.0
>Organization:
NetBSD
>Environment:
NetBSD 10.0 amd64
>Description:
packets from some of my blocklist addresses that are appended in variables passes.
192.168.100.8 passes but those from 192.168.100.5 rightly gets blocked.
>How-To-Repeat:
$home = { 192.168.100.5, 192.168.100.8}
$office = {192.168.64.3, 192.168.64.9}
$blocklist = {$home, $office }
group "external" on wm0 {
block final from $blocklist to any port ssh
}
group default {
pass all
}
>Fix:
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: bin-bug-people->joe
Responsible-Changed-By: joe@NetBSD.org
Responsible-Changed-When: Sat, 05 Jul 2025 10:47:48 +0000
Responsible-Changed-Why:
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/59511: some variable addresses not processed by firewall rules.
Date: Sat, 5 Jul 2025 15:34:13 -0000 (UTC)
emmankoko519@gmail.com writes:
>>How-To-Repeat:
>$home = { 192.168.100.5, 192.168.100.8}
>$office = {192.168.64.3, 192.168.64.9}
>$blocklist = {$home, $office }
$blocklist is a list of two VAR_ID elements.
This gets resolved in npf_var.c:263
/*
* Resolve if it is a reference to another variable.
*/
if (el->e_type == NPFVAR_VAR_ID) {
const npfvar_t *rvp = npfvar_lookup(el->e_data);
return npfvar_get_element(rvp, 0, level + 1);
}
but which fetches only index 0 of the element list stored in a variable.
As a result you get { 192.168.100.5, 192.168.64.3 }.
npfvar_get_element() either needs to handle such recursive lists or you
need to derefence variables when constructing lists. The latter probably
has more side effects.
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/59511: some variable addresses not processed by firewall rules.
Date: Sat, 5 Jul 2025 16:28:25 -0000 (UTC)
emmankoko519@gmail.com writes:
>packets from some of my blocklist addresses that are appended in variables passes.
>192.168.100.8 passes but those from 192.168.100.5 rightly gets blocked.
Maybe this:
Index: usr.sbin/npf/npfctl/npf_var.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/npf/npfctl/npf_var.c,v
retrieving revision 1.15
diff -p -u -r1.15 npf_var.c
--- usr.sbin/npf/npfctl/npf_var.c 1 Jun 2025 00:54:36 -0000 1.15
+++ usr.sbin/npf/npfctl/npf_var.c 5 Jul 2025 16:26:30 -0000
@@ -57,6 +57,8 @@ struct npfvar {
void * v_next;
};
+static size_t npfvar_get_count1(const npfvar_t *, size_t);
+
static npfvar_t * var_list = NULL;
static size_t var_num = 0;
@@ -222,16 +224,47 @@ npf_var_rid(char *var_id, rid_parser par
}
}
+static size_t
+npfvar_get_count1(const npfvar_t *vp, size_t level)
+{
+ npf_element_t *el;
+ size_t count = 0;
+
+ if (vp == NULL) {
+ return 0;
+ }
+ if (level >= var_num) {
+ yyerror("circular dependency for variable '%s'", vp->v_key);
+ return 0;
+ }
+ el = vp->v_elements;
+ while (el) {
+ if (el->e_type == NPFVAR_VAR_ID) {
+ const npfvar_t *rvp;
+ rvp = npfvar_lookup(el->e_data);
+ if (rvp != NULL)
+ count += npfvar_get_count1(rvp, level + 1);
+ } else {
+ count += 1;
+ }
+ el = el->e_next;
+ }
+
+ return count;
+}
+
size_t
npfvar_get_count(const npfvar_t *vp)
{
- return vp ? vp->v_count : 0;
+ return npfvar_get_count1(vp, 0);
}
static npf_element_t *
npfvar_get_element(const npfvar_t *vp, size_t idx, size_t level)
{
npf_element_t *el;
+ size_t togo, total;
+ const npfvar_t *rvp;
/*
* Verify the parameters.
@@ -243,27 +276,40 @@ npfvar_get_element(const npfvar_t *vp, s
yyerror("circular dependency for variable '%s'", vp->v_key);
return NULL;
}
- if (vp->v_count <= idx) {
- yyerror("variable '%s' has only %zu elements, requested %zu",
- vp->v_key, vp->v_count, idx);
- return NULL;
- }
-
/*
* Get the element at the given index.
*/
el = vp->v_elements;
- while (idx--) {
+ rvp = NULL;
+ togo = idx;
+ total = 0;
+ while (el) {
+ /*
+ * Resolve if it is a reference to another variable.
+ */
+ if (el->e_type == NPFVAR_VAR_ID) {
+ rvp = npfvar_lookup(el->e_data);
+ if (rvp != NULL && rvp->v_count > 0) {
+ if (togo < rvp->v_count)
+ return npfvar_get_element(rvp,
+ togo, level + 1);
+ total += (rvp->v_count - 1);
+ togo -= (rvp->v_count - 1);
+ }
+ }
+
+ total += 1;
+ if (togo-- == 0)
+ break;
+
el = el->e_next;
}
- /*
- * Resolve if it is a reference to another variable.
- */
- if (el->e_type == NPFVAR_VAR_ID) {
- const npfvar_t *rvp = npfvar_lookup(el->e_data);
- return npfvar_get_element(rvp, 0, level + 1);
+ if (el == NULL) {
+ yyerror("variable '%s' has only %zu elements, requested %zu",
+ vp->v_key, total, idx);
}
+
return el;
}
From: Emmanuel Nyarko <emmankoko519@gmail.com>
To: gnats-bugs@netbsd.org
Cc: Emmanuel <joe@netbsd.org>,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: bin/59511: some variable addresses not processed by firewall
rules.
Date: Mon, 7 Jul 2025 08:55:23 +0000
Hi Michael,
Thanks for the pointers and the patch. I will make make use of it.=20
Thanks again!!
> On 5 Jul 2025, at 4:30=E2=80=AFPM, Michael van Elst via gnats =
<gnats-admin@NetBSD.org> wrote:
>=20
> The following reply was made to PR bin/59511; it has been noted by =
GNATS.
>=20
> From: mlelstv@serpens.de (Michael van Elst)
> To: gnats-bugs@netbsd.org
> Cc:=20
> Subject: Re: bin/59511: some variable addresses not processed by =
firewall rules.
> Date: Sat, 5 Jul 2025 16:28:25 -0000 (UTC)
>=20
> emmankoko519@gmail.com writes:
>=20
>> packets from some of my blocklist addresses that are appended in =
variables passes.
>> 192.168.100.8 passes but those from 192.168.100.5 rightly gets =
blocked.
>=20
>=20
> Maybe this:
>=20
>=20
> Index: usr.sbin/npf/npfctl/npf_var.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> RCS file: /cvsroot/src/usr.sbin/npf/npfctl/npf_var.c,v
> retrieving revision 1.15
> diff -p -u -r1.15 npf_var.c
> --- usr.sbin/npf/npfctl/npf_var.c 1 Jun 2025 00:54:36 -0000 1.15
> +++ usr.sbin/npf/npfctl/npf_var.c 5 Jul 2025 16:26:30 -0000
> @@ -57,6 +57,8 @@ struct npfvar {
> void * v_next;
> };
>=20
> +static size_t npfvar_get_count1(const npfvar_t *, size_t);
> +
> static npfvar_t * var_list =3D NULL;
> static size_t var_num =3D 0;
>=20
> @@ -222,16 +224,47 @@ npf_var_rid(char *var_id, rid_parser par
> }
> }
>=20
> +static size_t
> +npfvar_get_count1(const npfvar_t *vp, size_t level)
> +{
> + npf_element_t *el;
> + size_t count =3D 0;
> +
> + if (vp =3D=3D NULL) {
> + return 0;
> + }
> + if (level >=3D var_num) {
> + yyerror("circular dependency for variable '%s'", vp->v_key);
> + return 0;
> + }
> + el =3D vp->v_elements;
> + while (el) {
> + if (el->e_type =3D=3D NPFVAR_VAR_ID) {
> + const npfvar_t *rvp;
> + rvp =3D npfvar_lookup(el->e_data);
> + if (rvp !=3D NULL)
> + count +=3D npfvar_get_count1(rvp, level + 1);
> + } else {
> + count +=3D 1;
> + }
> + el =3D el->e_next;
> + }
> +
> + return count;
> +}
> +
> size_t
> npfvar_get_count(const npfvar_t *vp)
> {
> - return vp ? vp->v_count : 0;
> + return npfvar_get_count1(vp, 0);
> }
>=20
> static npf_element_t *
> npfvar_get_element(const npfvar_t *vp, size_t idx, size_t level)
> {
> npf_element_t *el;
> + size_t togo, total;
> + const npfvar_t *rvp;
>=20
> /*
> * Verify the parameters.
> @@ -243,27 +276,40 @@ npfvar_get_element(const npfvar_t *vp, s
> yyerror("circular dependency for variable '%s'", vp->v_key);
> return NULL;
> }
> - if (vp->v_count <=3D idx) {
> - yyerror("variable '%s' has only %zu elements, requested %zu",
> - vp->v_key, vp->v_count, idx);
> - return NULL;
> - }
> -
> /*
> * Get the element at the given index.
> */
> el =3D vp->v_elements;
> - while (idx--) {
> + rvp =3D NULL;
> + togo =3D idx;
> + total =3D 0;
> + while (el) {
> + /*
> + * Resolve if it is a reference to another variable.
> + */
> + if (el->e_type =3D=3D NPFVAR_VAR_ID) {
> + rvp =3D npfvar_lookup(el->e_data);
> + if (rvp !=3D NULL && rvp->v_count > 0) {
> + if (togo < rvp->v_count)
> + return npfvar_get_element(rvp,
> + togo, level + 1);
> + total +=3D (rvp->v_count - 1);
> + togo -=3D (rvp->v_count - 1);
> + }
> + }
> +
> + total +=3D 1;
> + if (togo-- =3D=3D 0)
> + break;
> +
> el =3D el->e_next;
> }
>=20
> - /*
> - * Resolve if it is a reference to another variable.
> - */
> - if (el->e_type =3D=3D NPFVAR_VAR_ID) {
> - const npfvar_t *rvp =3D npfvar_lookup(el->e_data);
> - return npfvar_get_element(rvp, 0, level + 1);
> + if (el =3D=3D NULL) {
> + yyerror("variable '%s' has only %zu elements, requested %zu",
> + vp->v_key, total, idx);
> }
> +
> return el;
> }
>=20
>=20
Emmanuel
From: "Emmanuel" <joe@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/59511 CVS commit: src/usr.sbin/npf
Date: Wed, 20 Aug 2025 11:04:00 +0000
Module Name: src
Committed By: joe
Date: Wed Aug 20 11:03:59 UTC 2025
Modified Files:
src/usr.sbin/npf/npfctl: npf_build.c npf_var.c npf_var.h
src/usr.sbin/npf/npftest: npftest.conf
src/usr.sbin/npf/npftest/libnpftest: npf_rule_test.c
Log Message:
PR bin/59511
when extracting variables for filtering in NPF, allow the handler to
recursively extract all variables that might be present in the parent variable
to fully get all the filter elements present in them. this issue poses a security risk
as intruders can find their way into your machine if you intend to block them
but have their IPs in a nested variable with other IPs as well.
so this needs to be pulled up to 9, 10, 11
this fix has been reviewed by christos@ and martin@ and tests have been included.
To generate a diff of this commit:
cvs rdiff -u -r1.61 -r1.62 src/usr.sbin/npf/npfctl/npf_build.c
cvs rdiff -u -r1.15 -r1.16 src/usr.sbin/npf/npfctl/npf_var.c
cvs rdiff -u -r1.13 -r1.14 src/usr.sbin/npf/npfctl/npf_var.h
cvs rdiff -u -r1.17 -r1.18 src/usr.sbin/npf/npftest/npftest.conf
cvs rdiff -u -r1.25 -r1.26 \
src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2025
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home