Warn of potential security issue if preg_replace is used with /e flag

Nice idea, I give it a try.

Here we go :)

The sniff scans the pattern of all relevant preg_* functions.
Currently the sniff is limited to plain text patterns - if the pattern is provided by a variable, the sniff is skipped.
I think we can extend the sniff to provide a limited support for patterns in variables / constants but it won't be bulletproof.

I didn't push the change right away since I'd like to have some feedback first.

+++ b/Drupal/Sniffs/Semantics/PregSecuritySniff.php
@@ -0,0 +1,81 @@
+            $warn = 'Using the e flag in ' . $tokens[$stackPtr]['content'] . ' is a possible security risk. For details see http://drupal.org/node/750148';
+            $phpcsFile->addError($warn, $argument['start'], 'PregEFlag');

you should use %s placeholders in error messages, so that they can be easily overridden.

Will this ever raise any false positives? I always tried to keep drupalcs as clean as possible and to avoid wrongly detected "mistakes", because people tend to believe in the error reports and "correct" them without thinking. Coder module has quite a few security checks that often produce silly false positives.

But otherwise looks good to me. Glad you found my generic function call sniff class :-)

you should use %s placeholders in error messages, so that they can be easily overridden.

Patch adjusted.

Will this ever raise any false positives?

Well I guess if someone really knows what he's doing the "e" flag is save = false positive.
But I don't see a need to use the "e" flag at all since the documentation says: It is generally possible to instead use preg_replace_callback() to transform the matches and generate a replacement string while avoiding the risk that user input may be executed as PHP.

Talked with klausi @drupalcon about the sniff -> agreed on committing it. Done in: http://drupalcode.org/project/drupalcs.git/commit/3fff0bb8780070c05e88a9...

Code sniffer warns even if no 'e' flag, for example on this (because of the 'e' charf after the escaped '/'):

  preg_match_all('/<(a|img).*?(href|src)=(\'|")(http)?[^\'|"]*(sites\\/default\\/files\\/)([^\'|"]+)(\'|")/', $text, $matches);

Sorry, I've found, that this is a duplicate: #1779020: Invalid match for "Using the e flag in preg_match"