[BUG?] Problem whitelisting address containing '+'

[Mike Brudenell]

<x-flowed>
Greetings -

I'm playing with MailScanner 4.31.6 (with updated Message.pm for MyDoom-O)
and believe I have found a problem with the pattern matching used to
whitelist addresses.


The Bug
-------
For some of my testing I have been sending messages to 'plussed' e-mail
addresses, which are of the form:

    realusername+mailfoldername at mail-dev0.york.ac.uk

I needed to whitelist such an address and have been tearing my hair out
trying to work out why it hasn't been working.

My first attempt was to add the following line to
etc/rules/spam.whitelist.rules

    To:            extm61+abuse at mail-dev0.york.ac.uk       yes

but this didn't succeed in whitelisting this recipient address (and yes, I
stopped/restarted MailScanner before sending my test messages!).  I then
made MailScanner add the envelope 'To' information as a header just to
check that the recipient address was indeed the above: it was.

Next I tried using '*' for the domain name...

    To:            extm61+abuse@*       yes

but that didn't whitelist the recipient address either.

Finally I tried the Perl regular expression format for the pattern in the
spam.whitelist.rules file...

    To:            /^extm61\+abuse@/       yes

and found this worked fine.  My suspicion therefore is that the '+'
character in the username part of the address is failing to match against
the non-Perl patterns.


The Solution
------------
Checking the code in Config.pm reveals it converts some wildcard characters
as follows:

    .   -->  \.
    @   -->  \@
    *   -->  .*

but neglects to change the 'one or more of the previous item' + character.
The simple fix is to locate the RuleToRegExp function within Config.pm and
change this:

    # Now it's got an @ sign and something both sides of it
    # Change . into \., @ into \@, * into .*
    $rule =~ s/\@/\\@/g;
    $rule =~ s/\./\\./g;
    $rule =~ s/\*/.*/g;

to this:

    # Now it's got an @ sign and something both sides of it
    # Change . into \., @ into \@, + into \+, * into .*
    $rule =~ s/\@/\\@/g;
    $rule =~ s/\./\\./g;
    $rule =~ s/\+/\\+/g;
    $rule =~ s/\*/.*/g;

Or, delving into funkier use of Perl pattern matching and automatically
maintained variables (the '$&' variable contains the last-matched string),
this can be further simplified to make maintaining the set of characters to
be escaped with '\' easier:

    # Now it's got an @ sign and something both sides of it
    # Change . into \., @ into \@, + into \+ etc, and * into .*
    $rule =~ s/[@.+]/\\$&/g;
    $rule =~ s/\*/.*/g;


A Question
----------
But is this omitting to escape '+' characters in addresses a bug or
deliberate design decision?  [Julian?]

 *  if a bug then the above tweak fixes it (but are there any other
    metacharacters that should also be escaped?)

 *  if a design decision then please could a warning be added to the
    etc/rules/{README,EXAMPLES} files saying that use of '+' (and other
    metacharacters) probably won't have the intended effect unless escaped?
    possibly other metacharacters)

The problem is that applying the above fix will break the configuration of
anyone who has already discovered the problem and is manually escaping the
'+' character in the address patterns within their rules files.  (But how
many people will be affected by this?)


Cheers,

Mike Brudenell

--
The Computing Service, University of York, Heslington, York Yo10 5DD, UK
Tel:+44-1904-433811  FAX:+44-1904-433740

* Unsolicited commercial e-mail is NOT welcome at this e-mail address. *

-------------------------- MailScanner list ----------------------
To leave, send    leave mailscanner    to jiscmail at jiscmail.ac.uk
Before posting, please see the Most Asked Questions at
http://www.mailscanner.biz/maq/     and the archives at
http://www.jiscmail.ac.uk/lists/mailscanner.html
</x-flowed>