[iptables PATCH 0/2] libxtables: Fix two xtoptions bugs

[iptables PATCH 0/2] libxtables: Fix two xtoptions bugs

[Phil Sutter]

While the first one is clearly a bug in my recent code deduplication
effort, the second one may be reckoned a missing feature.
XTTYPE_HOSTMASK is but used in many places and nothing claims the masks
must be contiguous.

Phil Sutter (2):
  libxtables: xtoptions: Fix for garbage access in
    xtables_options_xfrm()
  libxtables: xtoptions: Fix for non-CIDR-compatible hostmasks

 libxtables/xtoptions.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 
2.41.0

[iptables PATCH 1/2] libxtables: xtoptions: Fix for garbage access in xtables_options_xfrm()

[Phil Sutter]

Allocation of the temporary array did not account for a terminating NULL
entry, causing array boundary overstepping in the called
xtables_merge_options(), causing spurious errors in extension parameter
parsing.

Fixes: ed8c3ea4015f0 ("libxtables: Combine the two extension option mergers")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 libxtables/xtoptions.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libxtables/xtoptions.c b/libxtables/xtoptions.c
index e27223e339cb2..433a686c2b595 100644
--- a/libxtables/xtoptions.c
+++ b/libxtables/xtoptions.c
@@ -93,12 +93,13 @@ xtables_options_xfrm(struct option *orig_opts, struct option *oldopts,
 	for (num_new = 0; entry[num_new].name != NULL; ++num_new)
 		;
 
-	mp = xtables_calloc(num_new, sizeof(*mp));
+	mp = xtables_calloc(num_new + 1, sizeof(*mp));
 	for (i = 0; i < num_new; i++) {
 		mp[i].name	= entry[i].name;
 		mp[i].has_arg	= entry[i].type != XTTYPE_NONE;
 		mp[i].val	= entry[i].id;
 	}
+
 	merge = xtables_merge_options(orig_opts, oldopts, mp, offset);
 
 	free(mp);
-- 
2.41.0

[iptables PATCH 2/2] libxtables: xtoptions: Fix for non-CIDR-compatible hostmasks

[Phil Sutter]

In order to parse the mask, xtopt_parse_hostmask() calls
xtopt_parse_plenmask() thereby limiting netmask support to prefix
lengths (alternatively specified in IP address notation).

In order to lift this impractical restriction, make
xtopt_parse_plenmask() aware of the fact that xtopt_parse_plen() may
fall back to xtopt_parse_mask() which correctly initializes val.hmask
itself and indicates non-CIDR-compatible masks by setting val.hlen to
-1.

So in order to support these odd masks, it is sufficient for
xtopt_parse_plenmask() to skip its mask building from val.hlen value and
take whatever val.hmask contains.

Fixes: 66266abd17adc ("libxtables: XTTYPE_HOSTMASK support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 libxtables/xtoptions.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libxtables/xtoptions.c b/libxtables/xtoptions.c
index 433a686c2b595..02025e99d4832 100644
--- a/libxtables/xtoptions.c
+++ b/libxtables/xtoptions.c
@@ -692,6 +692,10 @@ static void xtopt_parse_plenmask(struct xt_option_call *cb)
 
 	xtopt_parse_plen(cb);
 
+	/* may not be convertible to CIDR notation */
+	if (cb->val.hlen == (uint8_t)-1)
+		goto out_put;
+
 	memset(mask, 0xFF, sizeof(union nf_inet_addr));
 	/* This shifting is AF-independent. */
 	if (cb->val.hlen == 0) {
@@ -712,6 +716,7 @@ static void xtopt_parse_plenmask(struct xt_option_call *cb)
 	mask[1] = htonl(mask[1]);
 	mask[2] = htonl(mask[2]);
 	mask[3] = htonl(mask[3]);
+out_put:
 	if (entry->flags & XTOPT_PUT)
 		memcpy(XTOPT_MKPTR(cb), mask, sizeof(union nf_inet_addr));
 }
-- 
2.41.0

Re: [iptables PATCH 0/2] libxtables: Fix two xtoptions bugs

[Phil Sutter]

On Tue, Nov 28, 2023 at 10:26:29PM +0100, Phil Sutter wrote:
> While the first one is clearly a bug in my recent code deduplication
> effort, the second one may be reckoned a missing feature.
> XTTYPE_HOSTMASK is but used in many places and nothing claims the masks
> must be contiguous.
> 
> Phil Sutter (2):
>   libxtables: xtoptions: Fix for garbage access in
>     xtables_options_xfrm()
>   libxtables: xtoptions: Fix for non-CIDR-compatible hostmasks

Series applied.