Subject: Re: Insecure dependency in eval
To: None <tech-pkg@netbsd.org>
From: Roland Illig <rillig@NetBSD.org>
List: tech-pkg
Date: 11/22/2005 21:29:18
This is a multi-part message in MIME format.
--------------030003040702050001060306
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Roland Illig wrote:
> I'm going to prepare a good patch and put that up here for discussion.
And here it is. I don't think it is security-related since the name of
the file indicates that it is only used for reading configuration files.
I have already reported it upstream.
Roland
--------------030003040702050001060306
Content-Type: text/plain;
name="patch-ba"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="patch-ba"
$NetBSD$
--- lib/Mail/SpamAssassin/Conf/Parser.pm.orig Fri Aug 12 02:38:46 2005
+++ lib/Mail/SpamAssassin/Conf/Parser.pm Tue Nov 22 14:41:09 2005
@@ -871,53 +871,35 @@ sub is_delimited_regexp_valid {
sub is_regexp_valid {
my ($self, $name, $re) = @_;
+ my ($saferetext);
# 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 $safere = $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/\Q$1\E([a-z]*)$//; $mods = $1;
- }
- elsif ($re =~ s/^\/(.*)\/([a-z]*)$/$1/) {
- $mods = $2;
- }
- else {
- $safere = "m#".$re."#";
- }
-
- # now prepend the modifiers, in order to check if they're valid
- if ($mods) {
- $re = "(?".$mods.")".$re;
- }
-
- # 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; }) {
-
- # now double-check -- try with the user-supplied delimiters as well
- my $evalstr = '("" =~ '.$safere.'); 1;';
- if (eval $evalstr) {
- return 1;
- }
+ if ($re =~ qr"^m(\{)(.*)\}([a-z]*)$"
+ || $re =~ qr"^m(\()(.*)\)([a-z]*)$"
+ || $re =~ qr"^m(<)(.*)>([a-z]*)$"
+ || $re =~ qr"^m(\W)(.*)\1([a-z]*)$"
+ || $re =~ qr"^(/)(.*)/([a-z]*)$") {
+ $saferetext = "(?$3)$2";
+
+ } elsif ($re =~ qr"^(.*)$") {
+ # untaint $re.
+ $saferetext = $1;
+
+ } else {
+ # cannot happen.
+ die;
+ }
+
+ my $safere;
+ if (eval { $safere = qr"${saferetext}"; 1; }) {
+ return $safere;
}
my $err = $@;
$err =~ s/ at .*? line \d.*$//;
- warn "config: invalid regexp for rule $name: $origre: $err\n";
+ warn "config: invalid regexp for rule $name: $re: $err\n";
$self->{conf}->{errors}++;
return 0;
}
--------------030003040702050001060306--