Skip to content

Commit a34af00

Browse files
committed
Merge branch 'master' into type-juggle
2 parents f8cd554 + 66845e0 commit a34af00

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+248
-138
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
/vendor
2+
composer.lock

Dockerfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ RUN addgroup -S tool && adduser -S -G tool tool && \
88
# Install phpcs-security-audit
99
RUN composer global require pheromone/phpcs-security-audit
1010
WORKDIR /tmp
11-
RUN sh ./vendor/pheromone/phpcs-security-audit/symlink.sh
1211

1312
# change user
1413
USER tool

README.md

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
phpcs-security-audit v2
1+
phpcs-security-audit v3
22
=======================
33

44
About
@@ -7,35 +7,29 @@ phpcs-security-audit is a set of [PHP_CodeSniffer](https://github.com/squizlabs/
77

88
It currently has core PHP rules as well as Drupal 7 specific rules.
99

10-
The tool also checks for CVE issues and security advisories related to CMS/framework. Using it, you can follow the versioning of components during static code analysis.
10+
The tool also checks for CVE issues and security advisories related to the CMS/framework. This enable you to follow the versioning of components during static code analysis.
1111

12-
The main reason of this project for being an extension of PHP_CodeSniffer is to have easy integration into continuous integration systems. It is also able to find security bugs that are not detected with object oriented analysis (like in [RIPS](http://rips-scanner.sourceforge.net/) or [PHPMD](http://phpmd.org/)).
12+
The main reason of this project for being an extension of PHP_CodeSniffer is to have easy integration into continuous integration systems. It is also able to find security bugs that are not detected with some object oriented analysis (such as [PHPMD](http://phpmd.org/)).
1313

14-
phpcs-security-audit is backed by [Floe design + technologies](https://floedesign.ca/) and written by [Jonathan Marcil](https://twitter.com/jonathanmarcil).
14+
phpcs-security-audit in its beginning was backed by Pheromone (later on named Floe design + technologies) and written by [Jonathan Marcil](https://twitter.com/jonathanmarcil).
1515

16-
[<img src="https://floedesign.ca/img/thumbs/floe.jpg" alt="Floe design + technologies" width="100">](https://floedesign.ca/)
1716

1817

1918
Install
2019
-------
2120

2221
Requires [PHP CodeSniffer](http://pear.php.net/package/PHP_CodeSniffer/) version 3.x with PHP 5.4 or higher.
2322

24-
Because of the way PHP CodeSniffer works, you need to put the `Security/` folder from phpcs-security-audit in `/usr/share/php/PHP/CodeSniffer/Standards` or do a symlink to it.
25-
26-
The easiest way to install is to git clone and use composer that will create the symlink for you:
27-
```
28-
composer install
29-
./vendor/bin/phpcs --standard=example_base_ruleset.xml tests.php
30-
```
31-
32-
The package is also on [Packagist](https://packagist.org/packages/pheromone/phpcs-security-audit):
23+
The easiest way to install is using [Composer](https://getcomposer.org/):
3324
```
34-
composer require pheromone/phpcs-security-audit
35-
sh vendor/pheromone/phpcs-security-audit/symlink.sh
25+
#WARNING: this currently doesn't work up until the v3 package is released
26+
#See Contribute section bellow for git clone instruction
27+
composer require --dev pheromone/phpcs-security-audit
3628
./vendor/bin/phpcs --standard=./vendor/pheromone/phpcs-security-audit/example_base_ruleset.xml ./vendor/pheromone/phpcs-security-audit/tests.php
3729
```
3830

31+
This will also install the [DealerDirect Composer PHPCS plugin](https://github.com/Dealerdirect/phpcodesniffer-composer-installer/) which will register the `Security` standard with PHP_CodeSniffer.
32+
3933
If you want to integrate it all with Jenkins, go see http://jenkins-php.org/ for extensive help.
4034

4135

@@ -44,14 +38,14 @@ Usage
4438

4539
Simply point to any XML ruleset file and a folder:
4640
```
47-
phpcs --extensions=php,inc,lib,module,info --standard=example_base_ruleset.xml /your/php/files/
41+
phpcs --extensions=php,inc,lib,module,info --standard=./vendor/pheromone/phpcs-security-audit/example_base_ruleset.xml /your/php/files/
4842
```
4943

5044
Specifying extensions is important since for example PHP code is within .module files in Drupal.
5145

5246
To have a quick example of output you can use the provided tests.php file:
5347
```
54-
$ phpcs --extensions=php,inc,lib,module,info --standard=example_base_ruleset.xml tests.php
48+
$ phpcs --extensions=php,inc,lib,module,info --standard=./vendor/pheromone/phpcs-security-audit/example_base_ruleset.xml ./vendor/pheromone/phpcs-security-audit/tests.php
5549
5650
FILE: tests.php
5751
--------------------------------------------------------------------------------
@@ -85,7 +79,7 @@ These global parameters are used in many rules:
8579
* ParanoiaMode: set to 1 to add more checks. 0 for less.
8680
* CmsFramework: set to the name of a folder containings rules and Utils.php (such as Drupal7, Symfony2).
8781

88-
They can be setted in the XML files or in command line for permanent config with `--config-set` or at runtime with `--runtime-set`. Note that the XML override all CLI options so remove it if you want to use it. The CLI usage is as follow `phpcs --runtime-set ParanoiaMode 0 --extensions=php --standard=example_base_ruleset.xml tests.php`;
82+
They can be set in a custom ruleset `phpcs.xml[.dist]` XML file or from the command line for permanent config with `--config-set` or at runtime with `--runtime-set`. Note that the XML override all CLI options so remove it if you want to use it. The CLI usage is as follow `phpcs --runtime-set ParanoiaMode 0 --extensions=php --standard=./vendor/pheromone/phpcs-security-audit/example_base_ruleset.xml tests.php`;
8983

9084
In some case you can force the paranoia mode on or off with the parameter `forceParanoia` inside the XML rule.
9185

@@ -121,13 +115,45 @@ You are not required to do your own sniffs for the modification to be useful, si
121115
If you implement any public cms/framework customization please make a pull request to help the project grows.
122116

123117

118+
Contribute
119+
----------
120+
It is possible to install with a `git clone` and play with it in the same folder.
121+
```
122+
composer install
123+
./vendor/bin/phpcs --standard=example_base_ruleset.xml --extensions=php tests.php
124+
```
125+
126+
By default it should set PHPCS to look in the current folder:
127+
```
128+
PHP CodeSniffer Config installed_paths set to ../../../
129+
```
130+
131+
If for any reason you need to change this (should work out of the box) you will need to `phpcs --config-set installed_paths` as explained in [PHP_CodeSniffer docs](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#setting-the-installed-standard-paths).
132+
133+
Master can contain breaking changes, so people are better to rely on releases for stable versions.
134+
135+
Those release packages are available [here on GitHub](releases) or on [Packagist](https://packagist.org/packages/pheromone/phpcs-security-audit).
136+
137+
Some guidelines if you want to create new rules:
138+
* Ensure that `ParanoiaMode` controls how verbose your sniff is
139+
* If sometime the sniff is a valid security concern, run it when paranoia=true only
140+
* Warnings are generally issued instead of Errors for most-of-the-time when paranoia=false
141+
* Errors are always generated when you are use about user input being used
142+
* Prefer false positives (annoying results) over false negatives (missing results)
143+
* paranoia=false should solve false positive, otherwise warn on anything remotely suspicious
144+
* Include at least one test that trigger your sniff into `tests.php`
145+
* Keep it as a one liner, doesn't need to make sense
146+
* Don't forget to include your new sniff in the `example_base_ruleset.xml` and `example_drupal7_ruleset.xml` when it applies.
147+
148+
124149
Annoyances
125150
----------
126151

127152
As any security tools, this one comes with it's share of annoyance. At first a focus on finding vulnerabilities will be done, but later it is planned to have a phase where efforts will be towards reducing annoyances, in particular with the number of false positives.
128153

129154
* It's a generator of false positives. This can actually help you learn what are the weak functions in PHP. Paranoia mode will fix that by doing a major cut-off on warnings when set to 0.
130-
* It's slow. On big Drupal modules and core it can take too much time (and RAM, reconfigure cli/php.ini to use 512M if needed) to run. Not sure if it's because of bugs in PHPCS or this set of rules, but will be investigated last. Meanwhile you can configure PHPCS to ignore big contrib modules (and run another instance of PHPCS for .info parsing only for them). An example is og taking hours, usually everything runs under 1-2 minutes and sometime around 5 minute. You can only use one core in PHP since no multithreading is available. Possible workaround is to use phpcs --ignore=folder to skip scanning of those parts.
155+
* This tool was created around 10 years ago. Some of its parts might look outdated, and support for old PHP code will still be present. The reality is that many code base scanned with it might be as old as the tool.
156+
* It's slow. On big Drupal modules and core it can take too much time (and RAM, reconfigure cli/php.ini to use 512M if needed) to run. Not sure if it's because of bugs in PHPCS or this set of rules, but will be investigated last. Meanwhile you can configure PHPCS to ignore big contrib modules (and run another instance of PHPCS for .info parsing only for them). An example is og taking hours, usually everything runs under 1-2 minutes and sometime around 5 minute. You can try using the `--parallel=8` (or another number) option to try and speed things up on supported OSes. Possible workaround is to use phpcs --ignore=folder to skip scanning of those parts.
131157
* For Drupal advisories checking: a module with multiple versions might be secure if a lesser fixed version exists and you'll still get the error or warning. Keep everything updated at latest as recommended on Drupal's website.
132158

133159

Security/Sniffs/BadFunctions/AssertsSniff.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -27,14 +27,20 @@ public function register() {
2727
*/
2828
public function process(File $phpcsFile, $stackPtr) {
2929
$tokens = $phpcsFile->getTokens();
30-
$utils = \PHPCS_SecurityAudit\Sniffs\UtilsFactory::getInstance();
30+
$utils = \PHPCS_SecurityAudit\Security\Sniffs\UtilsFactory::getInstance();
3131

3232
if ($tokens[$stackPtr]['content'] == 'assert') {
3333
$opener = $phpcsFile->findNext(T_OPEN_PARENTHESIS, $stackPtr, null, false, null, true);
3434
$closer = $tokens[$opener]['parenthesis_closer'];
3535
$s = $stackPtr + 1;
36-
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Sniffs\Utils::$staticTokens, array(T_STRING_CONCAT)), $s, $closer, true);
37-
if ($s) {
36+
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Security\Sniffs\Utils::$staticTokens, array(T_STRING_CONCAT)), $s, $closer, true);
37+
38+
// Accept true/false as the first parameter
39+
if (in_array(strtolower($tokens[$s]['content']), array('true', 'false'))) {
40+
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Security\Sniffs\Utils::$staticTokens, array(T_STRING_CONCAT)), $s + 1, $closer, true);
41+
}
42+
43+
if ($s) {
3844
$msg = 'Assert eval function ' . $tokens[$stackPtr]['content'] . '() detected with dynamic parameter';
3945
if ($utils::is_token_user_input($tokens[$s])) {
4046
$phpcsFile->addError($msg . ' directly from user input', $stackPtr, 'ErrFunctionHandling');

Security/Sniffs/BadFunctions/BackticksSniff.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -26,7 +26,7 @@ public function register() {
2626
* @return void
2727
*/
2828
public function process(File $phpcsFile, $stackPtr) {
29-
$utils = \PHPCS_SecurityAudit\Sniffs\UtilsFactory::getInstance();
29+
$utils = \PHPCS_SecurityAudit\Security\Sniffs\UtilsFactory::getInstance();
3030
$tokens = $phpcsFile->getTokens();
3131
$closer = $phpcsFile->findNext(T_BACKTICK, $stackPtr + 1, null, false, null, true);
3232
if (!$closer) {

Security/Sniffs/BadFunctions/CallbackFunctionsSniff.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -27,7 +27,7 @@ public function register() {
2727
*/
2828
public function process(File $phpcsFile, $stackPtr) {
2929
$tokens = $phpcsFile->getTokens();
30-
$utils = \PHPCS_SecurityAudit\Sniffs\UtilsFactory::getInstance();
30+
$utils = \PHPCS_SecurityAudit\Security\Sniffs\UtilsFactory::getInstance();
3131

3232
if (in_array($tokens[$stackPtr]['content'], $utils::getCallbackFunctions())) {
3333
$opener = $phpcsFile->findNext(T_OPEN_PARENTHESIS, $stackPtr, null, false, null, true);
@@ -41,7 +41,7 @@ public function process(File $phpcsFile, $stackPtr) {
4141
}
4242
}
4343
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens,
44-
\PHPCS_SecurityAudit\Sniffs\Utils::$staticTokens, array(T_STRING_CONCAT)), $s, $closer, true);
44+
\PHPCS_SecurityAudit\Security\Sniffs\Utils::$staticTokens, array(T_STRING_CONCAT)), $s, $closer, true);
4545
$msg = 'Function ' . $tokens[$stackPtr]['content'] . '() that supports callback detected';
4646
if ($s) {
4747
if ($utils::is_token_user_input($tokens[$s])) {

Security/Sniffs/BadFunctions/CryptoFunctionsSniff.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -25,7 +25,7 @@ public function register() {
2525
* @return void
2626
*/
2727
public function process(File $phpcsFile, $stackPtr) {
28-
$utils = \PHPCS_SecurityAudit\Sniffs\UtilsFactory::getInstance();
28+
$utils = \PHPCS_SecurityAudit\Security\Sniffs\UtilsFactory::getInstance();
2929
$tokens = $phpcsFile->getTokens();
3030
if (preg_match("/^mcrypt_/", $tokens[$stackPtr]['content']) || in_array($tokens[$stackPtr]['content'], $utils::getCryptoFunctions())) {
3131
$tokstr = $tokens[$stackPtr]['content'];

Security/Sniffs/BadFunctions/EasyRFISniff.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -26,7 +26,7 @@ public function register() {
2626
* @return void
2727
*/
2828
public function process(File $phpcsFile, $stackPtr) {
29-
$utils = \PHPCS_SecurityAudit\Sniffs\UtilsFactory::getInstance();
29+
$utils = \PHPCS_SecurityAudit\Security\Sniffs\UtilsFactory::getInstance();
3030
$tokens = $phpcsFile->getTokens();
3131
$s = $phpcsFile->findNext(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, $stackPtr, null, true, null, true);
3232

@@ -37,7 +37,7 @@ public function process(File $phpcsFile, $stackPtr) {
3737
$s = $stackPtr;
3838
}
3939
while ($s) {
40-
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Sniffs\Utils::$staticTokens), $s + 1, $closer, true);
40+
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Security\Sniffs\Utils::$staticTokens), $s + 1, $closer, true);
4141
if ($s && $utils::is_token_user_input($tokens[$s])) {
4242
if (\PHP_CodeSniffer\Config::getConfigData('ParanoiaMode') || !$utils::is_token_false_positive($tokens[$s], $tokens[$s+2])) {
4343
$phpcsFile->addError('Easy RFI detected because of direct user input with ' . $tokens[$s]['content'] . ' on ' . $tokens[$stackPtr]['content'], $s, 'ErrEasyRFI');

Security/Sniffs/BadFunctions/EasyXSSSniff.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -33,7 +33,7 @@ public function register() {
3333
* @return void
3434
*/
3535
public function process(File $phpcsFile, $stackPtr) {
36-
$utils = \PHPCS_SecurityAudit\Sniffs\UtilsFactory::getInstance();
36+
$utils = \PHPCS_SecurityAudit\Security\Sniffs\UtilsFactory::getInstance();
3737
if ($this->forceParanoia >= 0) {
3838
$parano = $this->forceParanoia ? 1 : 0;
3939
} else {
@@ -53,7 +53,7 @@ public function process(File $phpcsFile, $stackPtr) {
5353

5454
$warn = false;
5555
while ($s) {
56-
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Sniffs\Utils::$staticTokens), $s + 1, $closer, true);
56+
$s = $phpcsFile->findNext(array_merge(\PHP_CodeSniffer\Util\Tokens::$emptyTokens, \PHP_CodeSniffer\Util\Tokens::$bracketTokens, \PHPCS_SecurityAudit\Security\Sniffs\Utils::$staticTokens), $s + 1, $closer, true);
5757
if ($s && $utils::is_token_user_input($tokens[$s])) {
5858
$phpcsFile->addError('Easy XSS detected because of direct user input with ' . $tokens[$s]['content'] . ' on ' . $tokens[$stackPtr]['content'], $s, 'EasyXSSerr');
5959
} elseif ($s && $utils::is_XSS_mitigation($tokens[$s]['content'])) {

Security/Sniffs/BadFunctions/ErrorHandlingSniff.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?php
2-
namespace PHPCS_SecurityAudit\Sniffs\BadFunctions;
2+
namespace PHPCS_SecurityAudit\Security\Sniffs\BadFunctions;
33

44
use PHP_CodeSniffer\Sniffs\Sniff;
55
use PHP_CodeSniffer\Files\File;
@@ -27,7 +27,7 @@ public function register() {
2727
*/
2828
public function process(File $phpcsFile, $stackPtr) {
2929
$tokens = $phpcsFile->getTokens();
30-
$utils = new \PHPCS_SecurityAudit\Sniffs\Utils();
30+
$utils = new \PHPCS_SecurityAudit\Security\Sniffs\Utils();
3131

3232
if ($tokens[$stackPtr]['content'] == 'error_reporting') {
3333
$p = $utils::get_param_tokens($phpcsFile, $stackPtr, 1);

0 commit comments

Comments
 (0)