Mailing List Archive

Author: jm
Date: Wed May 11 20:07:35 2005
New Revision: 169749

URL: http://svn.apache.org/viewcvs?rev=169749&view=rev
Log:
ok, enough messing with is_regexp_valid. here's a new test case which exercises it extensively with a range of possible inputs, and a modified implementation that passes all those tests. Don't modify the code unless it passes the tests.

Added:
spamassassin/trunk/t/regexp_valid.t (with props)
Modified:
spamassassin/trunk/MANIFEST
spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm

Modified: spamassassin/trunk/MANIFEST
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/MANIFEST?rev=169749&r1=169748&r2=169749&view=diff
==============================================================================
--- spamassassin/trunk/MANIFEST (original)
+++ spamassassin/trunk/MANIFEST Wed May 11 20:07:35 2005
@@ -375,6 +375,7 @@
t/rcvd_parser.t
t/recips.t
t/recursion.t
+t/regexp_valid.t
t/relative_scores.t
t/report_safe.t
t/reportheader.t

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm?rev=169749&r1=169748&r2=169749&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm Wed May 11 20:07:35 2005
@@ -821,33 +821,43 @@
sub is_regexp_valid {
my ($self, $name, $re) = @_;

- $re =~ /^m?(\W)(.*)(?:\1|>|}|\)|\])(.*?)$/;
- my $pattern = $2;
- $pattern = "(?".$3.")".$pattern if $3;
+ # OK, try to remove any normal perl-style regexp delimiters at
+ # the start and end, and modifiers at the end if present,
+ # so we can validate those too.
+ my $origre = $re;
+ my $mods = '';
+ if ($re =~ s/^m{//) {
+ $re =~ s/}([a-z]*)$//; $mods = $1;
+ }
+ elsif ($re =~ s/^m\(//) {
+ $re =~ s/\)([a-z]*)$//; $mods = $1;
+ }
+ elsif ($re =~ s/^m<//) {
+ $re =~ s/>([a-z]*)$//; $mods = $1;
+ }
+ elsif ($re =~ s/^m(\W)//) {
+ $re =~ s/$1([a-z]*)$//; $mods = $1;
+ }
+ elsif ($re =~ s/^\/(.*)\/([a-z]*)$/$1/) {
+ $mods = $2;
+ }
+
+ # now prepend the modifiers, in order to check if they're valid
+ if ($mods) {
+ $re = "(?".$mods.")".$re;
+ }

- # the first eval tells us if the regexp is safe
- # the second eval tells us if the delimiters are ok
- if (!defined ($pattern)) {
- warn "config: invalid regexp for rule $name: $re: missing or invalid delimiters\n";
- $self->{conf}->{errors}++;
- return 0;
- }
- elsif (eval { ("" =~ m{$pattern}); 1; }) {
- my $evalstr = '("" =~ ' . $re . '); 1;';
- if (eval $evalstr) {
- return 1;
- } else {
- my $err = $@;
- $err =~ s/ at .*? line \d+,//;
- warn "config: invalid regexp for rule $name: $re: $err\n";
- $self->{conf}->{errors}++;
- return 0;
- }
+ # note: this MUST use m/...${re}.../ in some form or another, ie.
+ # interpolation of the $re variable into a code regexp, in order to test the
+ # security of the regexp. simply using ("" =~ $re) will NOT do that, and
+ # will therefore open a hole!
+ if (eval { ("" =~ m#${re}#); 1; }) {
+ return 1;
}
else {
my $err = $@;
$err =~ s/ at .*? line \d+\.\n?//;
- warn "config: invalid regexp for rule $name: $re: $err\n";
+ warn "config: invalid regexp for rule $name: $origre: $err\n";
$self->{conf}->{errors}++;
return 0;
}

Added: spamassassin/trunk/t/regexp_valid.t
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/t/regexp_valid.t?rev=169749&view=auto
==============================================================================
--- spamassassin/trunk/t/regexp_valid.t (added)
+++ spamassassin/trunk/t/regexp_valid.t Wed May 11 20:07:35 2005
@@ -0,0 +1,61 @@
+#!/usr/bin/perl -w
+
+# test regexp validation
+
+BEGIN {
+ if (-e 't/test_dir') { # if we are running "t/rule_names.t", kluge around ...
+ chdir 't';
+ }
+
+ if (-e 'test_dir') { # running from test directory, not ..
+ unshift(@INC, '../blib/lib');
+ }
+}
+
+my $prefix = '.';
+if (-e 'test_dir') { # running from test directory, not ..
+ $prefix = '..';
+}
+
+use strict;
+use SATest; sa_t_init("regexp_valid");
+use Test;
+use Mail::SpamAssassin;
+use vars qw(%patterns %anti_patterns);
+
+# settings
+plan tests => 22;
+
+# initialize SpamAssassin
+my $sa = create_saobj({'dont_copy_prefs' => 1});
+$sa->init(0); # parse rules
+
+
+sub tryone {
+ my $re = shift;
+ return $sa->{conf}->{parser}->is_regexp_valid('test', $re);
+}
+
+ok tryone qr/foo bar/;
+ok tryone qr/foo bar/i;
+ok tryone qr/foo bar/is;
+ok tryone qr/foo bar/im;
+ok tryone qr!foo bar!im;
+ok tryone 'qr/foo bar/';
+ok tryone 'qr/foo bar/im';
+ok tryone 'qr!foo bar!';
+ok tryone 'qr!foo bar!im';
+ok tryone '/^foo bar$/';
+ok tryone '/foo bar/';
+ok tryone '/foo bar/im';
+ok tryone 'm!foo bar!is';
+ok tryone 'm{foo bar}is';
+ok tryone 'm(foo bar)is';
+ok tryone 'm<foo bar>is';
+ok tryone 'foo bar';
+ok tryone 'foo/bar';
+ok !tryone 'foo(bar';
+ok !tryone 'foo(?{1})bar';
+ok !tryone '/foo(?{1})bar/';
+ok !tryone 'm!foo(?{1})bar!';
+

Propchange: spamassassin/trunk/t/regexp_valid.t
------------------------------------------------------------------------------
svn:executable = *