Hacker News Reader

Comment by nly | Hacker News Reader

In Perl wouldn't the /e modifier live outside the replacement string, as part of the literal regexp, and therefore out of harms way? The issue here is the "/e" modifier in PHP can be injected in to a context where the programmer didn't expect it. The preg_replace function should take the eval modifier as a flag parameter.

Perl does have a ?{} eval expression, but it has to be enabled with an extra 'use' directive. The documentation also suggests enabling taint checking or "constrained evaluation within a Safe compartment".

Contrast this to PHP, where /e appears to be on by default and there's no mention of it whatsoever in the documentation, except in the change log to say it's deprecated in 5.5.

Also PCRE, which PHP uses, is a C library... so I doubt it has any eval functionality. PHP implements this on top of PCRE to mimic Perl.

No I'm afraid it's another shoddy PHP design decision to blame here.

View on HN

Cute rant. But PHP uses /e in exactly the same way: The /e is part of the literal regexp.

> Contrast this to PHP, where /e appears to be on by default

No, /e is not enabled by default.

> and there's no mention of it whatsoever in the documentation

Yah, that's because it's not on by default. Do you expect them to document every idea you make up?

> is a C library... so I doubt it has any eval functionality

pcre_callout()

> No I'm afraid it's another shoddy PHP design decision to blame here.

No I'm afraid it's just another ignorant PHP basher to blame here. Do you just like to make stuff up about PHP, then bash it?

PS. To anyone reading this: Don't use /e with untrusted input, it's not safe. But if must assign blame, then blame perl for it, not php.

  | Don't use /e with untrusted input

This isn't a case of someone running:

  preg_replace($VAR1 + "/e", $VAR2,'');

No one passed untrusted input to '/e.' It's a case of untrusted input being passed to preg_replace() in an insecure way, which allowed '/e' to be enabled. All of this is sort of irrelevant anyways. The developers did not add the preg_replace() function, the attacker did so as a way to eval code without directly calling eval() (which would be easy to spot in the code, since it should so rarely be called in practice).

> Cute rant.

It wasn't a rant.

> But PHP uses /e in exactly the same way: The /e is part of the literal regexp.

PHP doesn't have literal regexps as part of the language. The regexps in PHP are string arguments to the preg_* family of functions. Perl has literal regexps. e.g. $var =~ s/$pattern/$replacement/e. If you inject "/e" in to $replacement variable, what happens? I don't know, my Perl is rusty, but I'm thinking that that would be either an unevaluated context, or a syntax error.

> No, /e is not enabled by default.

Only as of 5.5. That doesn't make the decision to enable it by default in the past excusable.

> pcre_callout()

This doesn't look like it has anything to do with /e or code evaluation. It's a callback mechanism that users of the library can use. How it's used is irrelevant, the topic at hand is PHPs behaviour.

> To anyone reading this: Don't use /e with untrusted input, it's not safe.

You missed the point. Programmers can use preg_replace poorly and not know that /e even exists. That was one point in the article: that this little function has a surprising and potentially dangerous feature. Just saying "oh well, don't use untrusted input" is naive.

> PHP doesn't have literal regexps as part of the language.

It doesn't really change anything. Instead of using / to delimit the parts each part is sent as an argument to the function.

> Is/$pattern/$replacement/e. f you inject "/e" in to $replacement variable, what happens? I don't know

In PHP that regex would look like preg_replace("/$pattern/e", $replacement), so injecting a /e in the replacement doesn't do anything - it has to be in the pattern not in the replacement.

> Only as of 5.5. That doesn't make the decision to enable it by default in the past excusable.

No, it was never enabled by default. Why do you think it was enabled by default? That wouldn't even make sense.

> It's a callback mechanism that users of the library can use. How it's used is irrelevant, the topic at hand is PHPs behaviour.

How it's used is to - drumroll - enable /e - the callback runs the PHP function that executes the code whenever it sees a place where the /e should activate. i.e. /e is certainly part of the preg C library.

> Contrast this to PHP, where /e appears to be on by default and there's no mention of it whatsoever in the documentation, except in the change log to say it's deprecated in 5.5.

The preg_replace documentation says "Several PCRE modifiers are also available, including 'e' (PREG_REPLACE_EVAL), which is specific to this function" with a link to the PCRE modifiers page: http://de2.php.net/manual/en/reference.pcre.pattern.modifier.... This page has a bunch of warnings on the /e modifier :)

Generally putting user input into any regular expression function without running preg_quote over it first is a bad idea. Not just because of /e, but also various other issues, e.g. causing pathologically slow matches (DOS) or segfaults by deep recursion (DOS and maybe security relevant).

> No I'm afraid it's another shoddy PHP design decision to blame here.

Please keep in mind the context of the thread. The argument wasn't the implementation, but the decision to include eval functionality in replacement functionality. Something that exists in PCRE because of Perl. So unless you want to suggest PCRE doesn't have an /e modifier that performs an eval, you aren't really adding anything to the discussion.

Reply on news.ycombinator.com