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

[Mira Limbeck <m.limbeck@proxmox.com>]

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>