[pmg-devel] [PATCH pmg-api] ruledb: match field: improve handling of invalid regular expressions

[pmg-devel] [PATCH pmg-api] ruledb: match field: improve handling of invalid regular expressions

[Dominik Csapak]

by not saving them in the first place if they die during execution.
We test this by using them once on an empty string.

Since users may have saved already invalid ones, only warn if we encounter
such a regex in 'parse_entity' during execution instead of die'ing. Otherwise
pmg-smtp-filter will exit and restart, possibly leading to wrongly denying
mails (and possibly sending out NDRs) before spam checking was done.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/RuleDB/MatchField.pm | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
index 2b56058..177a283 100644
--- a/src/PMG/RuleDB/MatchField.pm
+++ b/src/PMG/RuleDB/MatchField.pm
@@ -69,7 +69,13 @@ sub save {
 
     defined($self->{ogroup}) || die "undefined ogroup: ERROR";
 
-    my $new_value = "$self->{field}:$self->{field_value}";
+    my $regex = $self->{field_value};
+
+    # test regex for validity
+    eval { "" =~ /$regex/i; };
+    die "invalid regex: $@\n" if $@;
+
+    my $new_value = "$self->{field}:$regex";
     $new_value =~ s/\\/\\\\/g;
     $new_value = encode('UTF-8', $new_value);
 
@@ -111,9 +117,12 @@ sub parse_entity {
 	    my $decvalue = PMG::Utils::decode_rfc1522($value);
 	    $decvalue = PMG::Utils::try_decode_utf8($decvalue);
 
-	    if ($decvalue =~ m|$self->{field_value}|i) {
-		push @$res, $id;
-	    }
+	    eval {
+		if ($decvalue =~ m|$self->{field_value}|i) {
+		    push @$res, $id;
+		}
+	    };
+	    warn "invalid regex: $@\n" if $@;
 	}
     }
 
-- 
2.30.2

Re: [pmg-devel] [PATCH pmg-api] ruledb: match field: improve handling of invalid regular expressions

[Mira Limbeck]

On 4/14/23 11:14, Dominik Csapak wrote:
> by not saving them in the first place if they die during execution.
> We test this by using them once on an empty string.
>
> Since users may have saved already invalid ones, only warn if we encounter
> such a regex in 'parse_entity' during execution instead of die'ing. Otherwise
> pmg-smtp-filter will exit and restart, possibly leading to wrongly denying
> mails (and possibly sending out NDRs) before spam checking was done.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>   src/PMG/RuleDB/MatchField.pm | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
> index 2b56058..177a283 100644
> --- a/src/PMG/RuleDB/MatchField.pm
> +++ b/src/PMG/RuleDB/MatchField.pm
> @@ -69,7 +69,13 @@ sub save {
>   
>       defined($self->{ogroup}) || die "undefined ogroup: ERROR";
>   
> -    my $new_value = "$self->{field}:$self->{field_value}";
> +    my $regex = $self->{field_value};
> +
> +    # test regex for validity
> +    eval { "" =~ /$regex/i; };
> +    die "invalid regex: $@\n" if $@;
> +
> +    my $new_value = "$self->{field}:$regex";
>       $new_value =~ s/\\/\\\\/g;
>       $new_value = encode('UTF-8', $new_value);
>   
> @@ -111,9 +117,12 @@ sub parse_entity {
>   	    my $decvalue = PMG::Utils::decode_rfc1522($value);
>   	    $decvalue = PMG::Utils::try_decode_utf8($decvalue);
>   
> -	    if ($decvalue =~ m|$self->{field_value}|i) {
> -		push @$res, $id;
> -	    }
> +	    eval {
> +		if ($decvalue =~ m|$self->{field_value}|i) {
> +		    push @$res, $id;
> +		}
> +	    };
> +	    warn "invalid regex: $@\n" if $@;
>   	}
>       }
>   

Tested adding:

* Match Field - fixed

* Content Type Filter - fixed

* Match Filename - not affected since it is anchored

* Archive Filter - fixed

* Match Archive Filename - not affected since it is anchored


pmg-smtp-filter no longer exits with errors and mail is passed through 
the rule system correctly with this patch.

previous log entries:
Apr 14 10:58:10 pmg pmg-smtp-filter[945]: Quantifier follows nothing in 
regex; marked by <-- HERE in m/* <-- HERE asdfas/ at 
/usr/share/perl5/PMG/RuleDB/MatchField.pm line 114.
Apr 14 10:58:10 pmg pmg-smtp-filter[945]: fast exit because of errors 
(free 300843008 bytes)
Apr 14 10:58:10 pmg postfix/lmtp[1029]: 513D338013E: 
to=<m.limbeck@proxmox.com>, relay=127.0.0.1[127.0.0.1]:10023, 
delay=0.11, delays=0.05/0/0.05/0.01, dsn=4.4.0, status=deferred (host 
127.0.0.1[127.0.0.1] said: 451 4.4.0 detected undelive>

new log entries:
Apr 14 12:04:58 pmg pmg-smtp-filter[6204]: WARNING: invalid regex: 
Quantifier follows nothing in regex; marked by <-- HERE in m/* <-- HERE 
asdfas/ at /usr/share/perl5/PMG/RuleDB/MatchField.pm line 121.
Apr 14 12:04:58 pmg pmg-smtp-filter[6204]: 38029D6439254AEA3F9: moved 
mail for <m.limbeck@proxmox.com> to spam quarantine - 
3804B76439254AEDA25 (rule: block all)
Apr 14 12:04:58 pmg pmg-smtp-filter[6204]: 38029D6439254AEA3F9: 
processing time: 0.021 seconds (0, 0.01, 0)


Tested-by: Mira Limbeck <m.limbeck@proxmox.com>

Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>

[pmg-devel] applied: [PATCH pmg-api] ruledb: match field: improve handling of invalid regular expressions

[Thomas Lamprecht]

Am 14/04/2023 um 11:14 schrieb Dominik Csapak:
> by not saving them in the first place if they die during execution.
> We test this by using them once on an empty string.
> 
> Since users may have saved already invalid ones, only warn if we encounter
> such a regex in 'parse_entity' during execution instead of die'ing. Otherwise
> pmg-smtp-filter will exit and restart, possibly leading to wrongly denying
> mails (and possibly sending out NDRs) before spam checking was done.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PMG/RuleDB/MatchField.pm | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
>

with Mira's R-b & T-b and slightly touching up commit message: applied, thanks!