[PATCH pve-firewall] fix #8261: config parser: check prefix length without Net::IP

[PATCH pve-firewall] fix #8261: config parser: check prefix length without Net::IP

[Arthur Bied-Charreton]

99343fe ("firewall: reject cidrs with invalid netmask") made
parse_ip_or_cidr call pve_verify_ip_or_cidr. That function masks the
host bits and constructs a Net::IP object for every alias and IPSet
entry, which is very expensive.

As noted in the linked bug, a cluster config with about 30k IPSet
entries took 6 seconds to parse vs. 0.4 before this call was introduced.

The address syntax is already checked by the IPV4RE and IPV6RE matches
in parse_ip_or_cidr and the host bits are masked away before being
passed to Net::IP in pve_verify_ip_or_cidr, so the only thing that call
really added is the prefix length bound. Check that bound directly on
the prefix instead.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=8261
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/Firewall.pm             | 10 ++++++----
 src/test/test_compile_ipsets.pl | 15 ++++++++++++++-
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 20e0250..467815c 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -3584,12 +3584,14 @@ sub parse_ip_or_cidr {
 
     my $ipversion;
 
-    pve_verify_ip_or_cidr($cidr);
-
-    if ($cidr =~ m!^(?:$IPV6RE)(/(\d+))?$!) {
+    if ($cidr =~ m!^(?:$IPV6RE)(/(?<prefix>\d+))?$!) {
+        die "prefix length $+{prefix} is longer than IP address (128)\n"
+            if defined($+{prefix}) && $+{prefix} > 128;
         $cidr =~ s|/128$||;
         $ipversion = 6;
-    } elsif ($cidr =~ m!^(?:$IPV4RE)(/(\d+))?$!) {
+    } elsif ($cidr =~ m!^(?:$IPV4RE)(/(?<prefix>\d+))?$!) {
+        die "prefix length $+{prefix} is longer than IP address (32)\n"
+            if defined($+{prefix}) && $+{prefix} > 32;
         $cidr =~ s|/32$||;
         $ipversion = 4;
     } else {
diff --git a/src/test/test_compile_ipsets.pl b/src/test/test_compile_ipsets.pl
index 593f5ee..4c7c446 100755
--- a/src/test/test_compile_ipsets.pl
+++ b/src/test/test_compile_ipsets.pl
@@ -9,11 +9,24 @@ use Test::More;
 
 use PVE::Firewall;
 
-for my $cidr ('192.0.2.0/99', '2001:db8::/129') {
+for my $cidr ('192.0.2.0/99', '10.0.0.0/33', '2001:db8::/129', '::/129') {
     eval { PVE::Firewall::parse_ip_or_cidr($cidr) };
     ok($@, "reject invalid CIDR $cidr");
 }
 
+my $valid = {
+    '10.0.0.0/0' => ['10.0.0.0/0', 4],
+    '10.0.0.1/32' => ['10.0.0.1', 4],
+    '10.0.0.1/032' => ['10.0.0.1/032', 4],
+    '::/0' => ['::/0', 6],
+    '::1/128' => ['::1', 6],
+    '::1/0128' => ['::1/0128', 6],
+};
+for my $cidr (sort keys $valid->%*) {
+    my @parsed = eval { PVE::Firewall::parse_ip_or_cidr($cidr) };
+    is_deeply(\@parsed, $valid->{$cidr}, "accept CIDR $cidr");
+}
+
 subtest 'container IP filters' => sub {
     plan skip_all => 'PVE::LXC::Config is not installed'
         if !eval { require PVE::LXC::Config; 1 };
-- 
2.47.3

applied: [PATCH pve-firewall] fix #8261: config parser: check prefix length without Net::IP

[Thomas Lamprecht]

On Tue, 03 Nov 2026 13:10:14 +0100, Arthur Bied-Charreton wrote:
> 99343fe ("firewall: reject cidrs with invalid netmask") made
> parse_ip_or_cidr call pve_verify_ip_or_cidr. That function masks the
> host bits and constructs a Net::IP object for every alias and IPSet
> entry, which is very expensive.
> 
> As noted in the linked bug, a cluster config with about 30k IPSet
> entries took 6 seconds to parse vs. 0.4 before this call was introduced.
> 
> [...]

Applied, thanks!

I folded in `\z` anchors, so a trailing newline stays rejected like with the
removed call, plus a test for that, and noted in the commit message that a
`/0` prefix with host bits set is now accepted. I also made the API accept
that in a follow-up, since clean_cidr treated a literal `/0` like a missing
prefix:

82888bc ("api: accept host bits in /0 networks like for other prefixes")

[1/1] fix #8261: config parser: check prefix length without Net::IP
      commit: eea1b88b9eaf8735197943ed2f4353a4a6715069