[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    Re: [oss-security]  Re: CVE Request - roundcubemail
From:       Florian Weimer <fw () deneb ! enyo ! de>
Date:       2008-12-28 10:26:12
Message-ID: 87eizsa94r.fsf () mid ! deneb ! enyo ! de
[Download RAW message or body]

* Steven M. Christey:

> On Wed, 17 Dec 2008, Florian Weimer wrote:
>
>> > I bet there's a chunk of these in various applications.  I believe Perl
>> > has similar functionality.
>>
>> Not quite, the s///e operator uses a compile-time transformation for
>> the replacement expression, so it shouldn't be affected by this very
>> issue.
>>
>> \Q \E pairs are an issue in the pattern, not the replacement.
>> Mistakes in this area increase the attack surface by exposing the
>> regular expression compiler to potentially hostile input, and it may
>> lead to denial-of-service vulnerabilities because some implementations
>> do not cope well with certain patterns.  Perhaps CWE-624 should be
>> split to reflect this?
>
> We'll take a closer look at it.

Thanks!

> I'm not exactly sure what you're saying here, though.  Do you mean that if
> attackers can insert a \Q or \E into the pattern, then they might be able
> to effectively modify the pattern in unexpected ways?

What I'm trying to say is: The PHP way of implementing
preg_replace("/$pattern/e", $expr, $subject) is something like this:

  my @captures = $subject =~ /$pattern/;
  if (@captures) {
    $expr =~ s/\$(\d+)/quotemeta($captures[$1])/ge; # expand captures
    $result = eval "$expr"; # run code
  } else {
    $result = $subject;
  }

This means that capture contents can leak into $expr and be executed.

Perl translates 

  $subject =~ s/$pattern/$expr/e;

to:

  BEGIN {
    eval "sub regexp001 {
      \$0 = \$_[0];
      \$1 = \$_[1];
      ... # number of assignments depends on \$expr
      $expr;
    }";
  }

  if ($subject =~ /$pattern/) {
    substr $subject, $-[0], $+[0] - $-[0], regexp001($1, $2, ...);
  }

Or something like that.  I can't find it in the source code, but it's
possible to reveal that the replacement expression is compiled early
by putting a BEGIN block into the replacement expression.
[prev in list] [next in list] [prev in thread] [next in thread]