You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@issues.apache.org on 2011/04/01 21:45:30 UTC
[Bug 6565] New: check_rbl_sub rules - all dots need to be escaped
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
Summary: check_rbl_sub rules - all dots need to be escaped
Product: Spamassassin
Version: SVN Trunk (Latest Devel Version)
Platform: All
OS/Version: All
Status: NEW
Severity: normal
Priority: P2
Component: Rules (Eval Tests)
AssignedTo: dev@spamassassin.apache.org
ReportedBy: Darxus@ChaosReigns.com
Everything of the from
eval:check_rbl_sub('<whatever>', '127.0.0.1')
should be replaced with
eval:check_rbl_sub('<whatever>', '^127\.0\.0\.1$')
"." means "any one character" in the perl regexes used to define these, and
these will match any substring in an IP if it doesn't start with "^" and end
with "$". So...
72_active.cf:header RCVD_IN_DNSWL_HI
eval:check_rbl_sub('dnswl-firsttrusted', '127.0.\d+.3')
will match 127.0.0.3, but it will also match 127.0.103.2. Which doesn't happen
because DNSWL doesn't currently have any three digit numbers in the third
octet.
I'm not seeing any cases where it looks like this is likely to currently be
causing problems.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #1 from Mark Martinec <Ma...@ijs.si> 2011-04-01 18:57:07 EDT ---
(> Everything of the from
> eval:check_rbl_sub('<whatever>', '127.0.0.1')
> should be replaced with
> eval:check_rbl_sub('<whatever>', '^127\.0\.0\.1$')
>
> "." means "any one character" in the perl regexes used to define these, and
> these will match any substring in an IP if it doesn't start with "^" and end
> with "$". So...
>
> 72_active.cf:header RCVD_IN_DNSWL_HI
> eval:check_rbl_sub('dnswl-firsttrusted', '127.0.\d+.3')
>
> will match 127.0.0.3, but it will also match 127.0.103.2.
> Which doesn't happen because DNSWL doesn't currently have
> any three digit numbers in the third octet.
>
> I'm not seeing any cases where it looks like this is likely to currently
> be causing problems.
Thanks for opening the ticket. Except for the first example, the rest is
correct, dns eval rules like '127.0.\d+.3' need fixing. Seems to be much
fewer of them than what I remembered.
What is not true is that an eval:check_rbl_sub(..., '127.0.0.1') needs
fixing. It doesn't, see Mail::SpamAssassin::Dns::process_dnsbl_set :
not all arguments to check_rbl_sub are regular expressions. The subroutine
tests first for exact equality, then checks for a single integer
(which implies a bitmask), and only if all this fails, the argument
is considered a regular expression. So, a '127.0.0.1' is alright, but
the '127.0.\d+.3' is not, it should have been a '^127\.0\.\d+\.3\z' (in
case of a leading 3-digit '127' the '^' anchor may arguably be omitted).
Similarly the eval tests in URIDNSBL.pm and in AskDNS.pm plugins are
quite sophisticated about their eval arguments, accepting several possible
syntaxes before falling back to interpreting it as a regular expression.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|Undefined |3.3.2
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #3 from Mark Martinec <Ma...@ijs.si> 2011-05-04 19:19:29 UTC ---
> Created attachment 4867 [details]
> Fix
>
> For 41 check_rbl_sub rules, changed regexes to exact equality, or added
> escapes, begin and end markers, where appropriate.
>
> Also made a minor correction to a DNSWL rule description.
Nice job, thanks! Fine with me to apply this to 3.3.
Seems you have missed the:
20_dnsbl_tests.cf
header RCVD_IN_XBL eval:check_rbl('zen-lastexternal', 'zen.spamhaus.org.',
'127.0.0.[45678]')
possibly others.
Actually what I wrote earlier:
> What is not true is that an eval:check_rbl_sub(..., '127.0.0.1') needs
> fixing. It doesn't, see Mail::SpamAssassin::Dns::process_dnsbl_set :
> not all arguments to check_rbl_sub are regular expressions. The subroutine
> tests first for exact equality, then checks for a single integer
> (which implies a bitmask), and only if all this fails, the argument
> is considered a regular expression.
...is how it is supposed to work and it's how it is documented.
Yet the implementation is wrong: if the exact match fails, it falls
onto a regexp test, even when a 'subtest' is a dotted-quad.
The attached patch should fix it.
trunk:
$ svn ci -m
'Bug 6565: The dotted-quad subtest should not be interpreted as a regexp'
Sending lib/Mail/SpamAssassin/Dns.pm
Committed revision 1099562.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] [review] check_rbl_sub rules - all dots need to be
escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|check_rbl_sub rules - all |[review] check_rbl_sub
|dots need to be escaped |rules - all dots need to be
| |escaped
Status Whiteboard| |needs 2 votes for 3.3.2
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] [review] check_rbl_sub rules - all dots need to be
escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kmcgrail@pccc.com
Status Whiteboard|needs 2 votes for 3.3.2 |needs 1 votes for 3.3.2
--- Comment #7 from Kevin A. McGrail <km...@pccc.com> 2011-05-04 20:52:52 UTC ---
+1 for 3.3.2 for KAM
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] [review] check_rbl_sub rules - all dots need to be
escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
Henrik Krohns <he...@hege.li> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |hege@hege.li
Status Whiteboard|needs 1 votes for 3.3.2 |ready to commit for 3.3.2
--- Comment #8 from Henrik Krohns <he...@hege.li> 2011-05-05 07:24:19 UTC ---
+1 for 3.3.2
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #6 from Mark Martinec <Ma...@ijs.si> 2011-05-04 20:33:50 UTC ---
> Fix v2
> Previous changes, plus two for SpamHaus.
Nice.
> Thanks for the catch. Apparently my grep for 127 was just in rulesrc, not
> rules. This should be all of them.
trunk:
$ svn ci -m 'Bug 6565: check_rbl_sub rules - all dots need to be escaped'
Sending rules/20_dnsbl_tests.cf
Sending rulesrc/sandbox/felicity/70_dnswl.cf
Sending rulesrc/sandbox/felicity/70_iadb.cf
Sending rulesrc/sandbox/wtogami/20_mailspike.cf
Committed revision 1099593.
Please vote (3.3.2) for the 'Fix v2'
plus my code fix (the second attachment).
+1 from me
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #5 from Darxus <Da...@ChaosReigns.com> 2011-05-04 19:58:26 UTC ---
Created attachment 4870
--> https://issues.apache.org/SpamAssassin/attachment.cgi?id=4870
Fix v2
Previous changes, plus two for SpamHaus.
Thanks for the catch. Apparently my grep for 127 was just in rulesrc, not
rules. This should be all of them.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #2 from Darxus <Da...@ChaosReigns.com> 2011-05-02 19:19:55 UTC ---
Created attachment 4867
--> https://issues.apache.org/SpamAssassin/attachment.cgi?id=4867
Fix
For 41 check_rbl_sub rules, changed regexes to exact equality, or added
escapes, begin and end markers, where appropriate.
Also made a minor correction to a DNSWL rule description.
I haven't figured out how to test most of these. How is promotion from
sandboxes supposed to happen?
http://wiki.apache.org/spamassassin/RulesProjPromotion says it's a manual
process, which I don't think is correct, and it also says that rules were moved
from trunk into a rules branch - where there's a readme that says it's not used
anymore.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] [review] check_rbl_sub rules - all dots need to be
escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #10 from Mark Martinec <Ma...@ijs.si> 2011-05-05 15:42:28 UTC ---
> The rulesrc/sandbox/felicity/70_dnswl.cf and
> rulesrc/sandbox/felicity/70_iadb.cf are copied from
> trunk, as they have an added 'reuse' clause and the
> patch did not apply cleanly to them.
Forgot to commit these two.
branch 3.3:
Bug 6565: check_rbl_sub rules - all dots need to be escaped - commit
felicity/70_dnswl.cf and felicity/70_iadb.cf too
Sending rulesrc/sandbox/felicity/70_dnswl.cf
Sending rulesrc/sandbox/felicity/70_iadb.cf
Committed revision 1099848.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] check_rbl_sub rules - all dots need to be escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
--- Comment #4 from Mark Martinec <Ma...@ijs.si> 2011-05-04 19:21:45 UTC ---
Created attachment 4869
--> https://issues.apache.org/SpamAssassin/attachment.cgi?id=4869
The dotted-quad subtest should not be interpreted as a regexp - a fix
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6565] [review] check_rbl_sub rules - all dots need to be
escaped
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6565
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #9 from Mark Martinec <Ma...@ijs.si> 2011-05-05 12:33:53 UTC ---
3.3 branch:
$ svn ci -m 'Bug 6565: check_rbl_sub rules - all dots need to be escaped'
Sending lib/Mail/SpamAssassin/Dns.pm
Sending rules/20_dnsbl_tests.cf
Committed revision 1099766.
Note that newer sandbox rules are only in the trunk, hence
the changes to rulesrc/sandbox/wtogami/20_mailspike.cf,
are not applicable to the 3.3 branch.
The rulesrc/sandbox/felicity/70_dnswl.cf and
rulesrc/sandbox/felicity/70_iadb.cf are copied from
trunk, as they have an added 'reuse' clause and the
patch did not apply cleanly to them.
These rule changes in the 3.3 branch are probably redundant,
but can't hurt.
Closing.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.