[PATCH pve-firewall 1/1] fix #7818: disallow line feeds in firewall comments

[PATCH pve-firewall 1/1] fix #7818: disallow line feeds 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.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Notes:
    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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 93f8c34..894a87b 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1228,6 +1228,18 @@ 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/[\n\r]/) {
+        die "comment must not contain a line feed\n";
+    }
+
+    return $comment;
+}
+
 # helper function for API
 
 sub copy_opject_with_digest {
@@ -1623,6 +1635,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 1/1] fix #7818: disallow line feeds in firewall comments

[Stoiko Ivanov]

Thanks for tackling this so quickly!

On Wed, 15 Jul 2026 11:34:30 +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.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> Notes:
>     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 | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 93f8c34..894a87b 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1228,6 +1228,18 @@ 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/[\n\r]/) {
suggestion: would restrict this here to printable characters
and exclude any vertical space:
https://perldoc.perl.org/perlunicode#%5Cp%7BPrint%7D
https://perldoc.perl.org/perlunicode#%5Cp%7BVertSpace%7D

as I'd not expect people to use control-chars in their comments
(of course that's breaking backward-compat a bit more)


> +        die "comment must not contain a line feed\n";
> +    }
> +
> +    return $comment;
> +}
> +
>  # helper function for API
>  
>  sub copy_opject_with_digest {
> @@ -1623,6 +1635,7 @@ my $rule_properties = {
>          description => "Descriptive comment.",
>          type => 'string',
>          optional => 1,
> +        format => 'pve-fw-comment-spec',
>      },
>      'icmp-type' => {
>          description =>

Re: [PATCH pve-firewall 1/1] fix #7818: disallow line feeds in firewall comments

[Stefan Hanreich]

On 7/15/26 11:48 AM, Stoiko Ivanov wrote:

[snip]

>> +sub pve_fw_verify_comment_spec {
>> +    my ($comment) = @_;
>> +
>> +    if ($comment =~ m/[\n\r]/) {
> suggestion: would restrict this here to printable characters
> and exclude any vertical space:
> https://perldoc.perl.org/perlunicode#%5Cp%7BPrint%7D
> https://perldoc.perl.org/perlunicode#%5Cp%7BVertSpace%7D
> 
> as I'd not expect people to use control-chars in their comments
> (of course that's breaking backward-compat a bit more)

Yeah, should've thought of that initially as well - sent a v2 addressing
that.

superseded: [PATCH pve-firewall 1/1] fix #7818: disallow line feeds in firewall comments

[Stefan Hanreich]

https://lore.proxmox.com/pve-devel/20260715101054.31295-1-s.hanreich@proxmox.com/T/#u

On 7/15/26 11:34 AM, Stefan Hanreich wrote:
> They do not get sanitized and interpreted literally, allowing for
> authenticated users to inject additional lines into the firewall
> configuration files.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> Notes:
>     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 | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 93f8c34..894a87b 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1228,6 +1228,18 @@ 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/[\n\r]/) {
> +        die "comment must not contain a line feed\n";
> +    }
> +
> +    return $comment;
> +}
> +
>  # helper function for API
>  
>  sub copy_opject_with_digest {
> @@ -1623,6 +1635,7 @@ my $rule_properties = {
>          description => "Descriptive comment.",
>          type => 'string',
>          optional => 1,
> +        format => 'pve-fw-comment-spec',
>      },
>      'icmp-type' => {
>          description =>