[PATCH pve-firewall v2 1/1] fix #7818: disallow vertical spaces in firewall comments

[PATCH pve-firewall v2 1/1] fix #7818: disallow vertical spaces in firewall comments

[Stefan Hanreich]

They do not get sanitized and interpreted literally, allowing for
authenticated users to inject additional lines into the firewall
configuration files. Additionally restrict the comments to only
printable characters which additionally prevents control characters in
comments.

Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Notes:
    Changes from v1 (Thanks @Stoiko):
    * disallow any vertical space instead of line feeds only
    * additionally only allow printable characters
    
    Did quickly check the other properties as well, but couldn't find
    anything. Will take a closer look still but wanted to get the patch
    ready in the meanwhile.

 src/PVE/Firewall.pm | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 93f8c34..8ec2b80 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1228,6 +1228,22 @@ sub pve_fw_verify_icmp_type_spec {
     return $icmp_type;
 }
 
+PVE::JSONSchema::register_format('pve-fw-comment-spec', \&pve_fw_verify_comment_spec);
+
+sub pve_fw_verify_comment_spec {
+    my ($comment) = @_;
+
+    if ($comment =~ m/\p{VertSpace}/) {
+        die "comment must not contain any line feeds\n";
+    }
+
+    if ($comment !~ m/^\p{Print}*$/) {
+        die "comment must only contain printable characters\n";
+    }
+
+    return $comment;
+}
+
 # helper function for API
 
 sub copy_opject_with_digest {
@@ -1623,6 +1639,7 @@ my $rule_properties = {
         description => "Descriptive comment.",
         type => 'string',
         optional => 1,
+        format => 'pve-fw-comment-spec',
     },
     'icmp-type' => {
         description =>
-- 
2.47.3

Re: [PATCH pve-firewall v2 1/1] fix #7818: disallow vertical spaces in firewall comments

[Stoiko Ivanov]

Thanks for the quick iteration.

gave it a minimal spin with a pvesh command i used to trigger the issue in
#7818 - catches the new-line as expected.

one nit inline - but with or without that:
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>

On Wed, 15 Jul 2026 12:10:53 +0200
Stefan Hanreich <s.hanreich@proxmox.com> wrote:

> They do not get sanitized and interpreted literally, allowing for
> authenticated users to inject additional lines into the firewall
> configuration files. Additionally restrict the comments to only
> printable characters which additionally prevents control characters in
> comments.
> 
> Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> Notes:
>     Changes from v1 (Thanks @Stoiko):
>     * disallow any vertical space instead of line feeds only
>     * additionally only allow printable characters
>     
>     Did quickly check the other properties as well, but couldn't find
>     anything. Will take a closer look still but wanted to get the patch
>     ready in the meanwhile.
> 
>  src/PVE/Firewall.pm | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 93f8c34..8ec2b80 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1228,6 +1228,22 @@ sub pve_fw_verify_icmp_type_spec {
>      return $icmp_type;
>  }
>  
> +PVE::JSONSchema::register_format('pve-fw-comment-spec', \&pve_fw_verify_comment_spec);
> +
> +sub pve_fw_verify_comment_spec {
> +    my ($comment) = @_;
> +
> +    if ($comment =~ m/\p{VertSpace}/) {
> +        die "comment must not contain any line feeds\n";
nit: would read "line feed" as `\n` only - not general vertical space.
but I think the message is expressive enough - and captures what went
wrong.


> +    }
> +
> +    if ($comment !~ m/^\p{Print}*$/) {
> +        die "comment must only contain printable characters\n";
> +    }
> +
> +    return $comment;
> +}
> +
>  # helper function for API
>  
>  sub copy_opject_with_digest {
> @@ -1623,6 +1639,7 @@ my $rule_properties = {
>          description => "Descriptive comment.",
>          type => 'string',
>          optional => 1,
> +        format => 'pve-fw-comment-spec',
>      },
>      'icmp-type' => {
>          description =>