Brian Wolff has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/395764 )
Change subject: Update README
......................................................................
Update README
Change-Id: I43bb5e9617e94e7e249e9b78702cde708043b202
---
M README
M src/TaintednessBaseVisitor.php
2 files changed, 100 insertions(+), 10 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/tools/phan/SecurityCheckPlugin
refs/changes/64/395764/1
diff --git a/README b/README
index 128b2fd..7b7ca3a 100644
--- a/README
+++ b/README
@@ -1,16 +1,106 @@
This is a plugin to Phan to try and detect security issues
-(such as XSS)
+(such as XSS). It keeps track of any time a user can modify
+a variable, and checks to see that such variables are
+escaped before being output as html or used as an sql query, etc.
-This is still experimental and very rough around the edges.
+It is primarily intended for scanning MediaWiki extensions,
+however it supports a generic mode which should work with
+any PHP project.
-If you want to use it add either GenericSecurityCheckPlugin.php
-or MediaWikiSecurityCheckPlugin.php to the plugins list in
-your phan config, depending on if you are checking a non-mediawiki
-project or a MediaWiki/MediaWiki extension respectively.
+This plugin should be considered beta quality. Generic mode isn't
+well tested yet.
-This is only tested with Phan 0.8.0
+Requirements:
+* php >= 7.0
+* Phan 0.8.0 [This has not been tested on any other version of phan]
+* Lots of memory. Scanning MediaWiki seems to take about 3 minutes
+and use about 2 GB of memory. Running out of memory may be a real issue
+if you try and scan something from within a VM that has limited
+memory.
-== Environment variables ==
-The following environment variables affect the plugin:
+== How to use ==
+=== via composer (recommended) ===
+[This doesn't actually work yet, once this plugin is in packagist]
+
+[TODO: fill this out]
+=== Manually ===
+
+For MediaWiki mode, add MediaWikiSecurityCheckPlugin.php to the
+list of plugins in your phan config.php file.
+
+For generic mode, add GenericSecurityCheckPlugin.php to the list
+of plugins in your phan config.php file.
+
+Then run phan as you normally would:
+$ php7.0 /path/to/phan/phan -p
+
+== Using the plugin ==
+
+The plugin will output various issue types depending on what it
+detects. The issue types it outputs are:
+
+* SecurityCheckMulti - For when there are multiple types of security issues
involved
+* SecurityCheck-XSS
+* SecurityCheck-SQLInjection
+* SecurityCheck-ShellInjection
+* SecurityCheck-PHPSerializeInjection - For when someone does unserialize(
$_GET['d'] ); This issue type seems to have a high false positive rate
currently.
+* SecurityCheck-CUSTOM1 - To allow people to have custom taint types
+* SecurityCheck-CUSTOM2 - ditto
+* SecurityCheck-OTHER - At the moment, this corresponds to things that don't
have an escaping function to make input safe. e.g. eval( $_GET['foo'] );
require $_GET['bar'];
+* SecurityCheck-LikelyFalsePositive - A potential issue, but probably not.
Mostly happens when the plugin gets confused.
+
+The severity field is usually marked as Issue::SEVERITY_NORMAL (5). False
positives
+get Issue::SEVERITY_LOW (0). Issues that may result in server compromise
+(as opposed to just end user compromise) such as shell or sql injection are
+marked as Issue::SEVERITY_CRITICAL (10). SerializationInjection would normally
+be "critical" but its currently denoted as a severity of 4 because the check
+seems to have a high false positive rate at the moment.
+
+You can use the -y command line option of phan to filter by severity.
+
+=== Limitations ===
+There's much more than listed here, but some notable limitations/bugs:
+
+* When an issue is output, the plugin tries to include details about what
+line originally caused the issue. Usually it works, but sometimes it gives
+misleading/wrong information
+** In particular, with pass by reference parameters to MediaWiki hooks,
sometimes
+the line number is the hook call in MediaWiki core, instead of the hook
+subscriber in the extension that caused the issue.
+* Command line scripts cause XSS false positives
+* The plugin won't recognize things that do custom escaping. If you have
+custom escaping methods, you may have to write a subclass of
SecurityCheckPlugin
+in order for the plugin to recognize it.
+
+=== Customizing ===
+The plugin supports being customized, by subclassing the SecurityCheckPlugin
+class. For a complex example of doing so, see MediaWikiSecurityCheckPlugin.
+
+You can add pretty much arbitrary behaviour here, but the primary thing
+you would usually want to customize is adding information about how different
+functions affect the taint of a variable.
+
+To do this, you override the getCustomFuncTaints() method. This method returns
+an associative array of fully qualified method names to an array describing
+how the taint of the return value of the function in terms of its arguments.
+The numeric keys correspond to the number of an argument, and an 'overall'
+key adds taint that is not present in any of the arguments. Basically for
+each argument, the plugin takes the taint of the argument, bitwise AND's it
+to its entry in the array, and then bitwise OR's the overall key. If any
+of the keys in the array have an EXEC flags, then an issue is immediately
+raised if the corresponding taint is fed the function (For example, an
+output function).
+
+For example, htmlspecialchars which removes html taint but leaves other taint
+would look like
+'htmlspecialchars' => [
+ self::YES_TAINT & ~self::HTML_TAINT,
+ 'overall' => self::NO_TAINT
+];
+
+=== Environment variables ===
+The following environment variables affect the plugin. Normally you would
+not have to adjust these.
+
* SECURITY_CHECK_EXT_PATH - Path to extension.json when in MediaWiki mode. If
not set assumes the project root directory.
* SECCHECK_DEBUG - File to output extra debug information (If running from
shell, /dev/stderr is convenient)
diff --git a/src/TaintednessBaseVisitor.php b/src/TaintednessBaseVisitor.php
index c63003b..340cad2 100644
--- a/src/TaintednessBaseVisitor.php
+++ b/src/TaintednessBaseVisitor.php
@@ -1443,7 +1443,7 @@
$issueType = 'SecurityCheck-PHPSerializeInjection';
// For now this is low because it seems to have a lot
// of false positives.
- $severity = Issue::SEVERITY_LOW;
+ $severity = 4;
} elseif (
$combinedTaint === SecurityCheckPlugin::CUSTOM1_TAINT
) {
--
To view, visit https://gerrit.wikimedia.org/r/395764
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I43bb5e9617e94e7e249e9b78702cde708043b202
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/tools/phan/SecurityCheckPlugin
Gerrit-Branch: master
Gerrit-Owner: Brian Wolff <bawolff...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
