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

[Stoiko Ivanov <s.ivanov@proxmox.com>]

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