[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 =>

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

[Fabian Grünbichler]

On Wed, 15 Jul 2026 11:34:30 +0200, Stefan Hanreich wrote:
> They do not get sanitized and interpreted literally, allowing for
> authenticated users to inject additional lines into the firewall
> configuration files.
> 
> 

Applied, thanks!

Went with this v1 instead of the v2, since it fixes the issue and the proposed
changes in v2 should be discussed, and if acked, applied wholesale not just to
this individual field.

[1/1] fix #7818: disallow line feeds in firewall comments
      commit: 97a07715161cea9fd403f473d3e19a6dc857d018

Best regards,
-- 
Fabian Grünbichler <f.gruenbichler@proxmox.com>