mediawiki / phan-taint-check-plugin
A Phan plugin to do security checking
Requires
- php: ^7.2.0 | ^8.0.0
- ext-json: *
- phan/phan: 3.2.6
Requires (Dev)
- mediawiki/mediawiki-codesniffer: 34.0.0
- mediawiki/minus-x: 1.1.0
- php-parallel-lint/php-console-highlighter: 0.5.0
- php-parallel-lint/php-parallel-lint: 1.2.0
- phpunit/phpunit: ^8.5
README
This is a plugin to Phan to try and detect security issues (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.
It supports generic PHP projects, and it also has a dedicated mode for MediaWiki code (analyzes hooks, HTMLForms and Database methods).
A web demo is available.
Usage
System requirements
php >= 7.2.0
Phan 3.2.6
- Strongly suggested:
php-ast >=1.0.1
. While this is not enforced via composer, using the fallback parser is way slower and more memory-draining than using php-ast. See https://github.com/nikic/php-ast for instructions. - Lots of memory. Scanning MediaWiki takes several GBs 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. Small projects do not require so much memory.
Install
$ composer require --dev mediawiki/phan-taint-check-plugin
Usage
The plugin can be used in both "manual" and "standalone" mode. The former is the best choice if your project is already running phan, and almost no configuration is needed. The latter should be used if you don't want to add phan to your project.
Manual
You simply have to add taint-check to the plugins
section of your phan config. Assuming
that taint-check is in the standard vendor location, e.g.
$seccheckPath = 'vendor/mediawiki/phan-taint-check-plugin/';
, the file to include is
"$seccheckPath/GenericSecurityCheckPlugin.php"
for a generic project, and
"$seccheckPath/MediaWikiSecurityCheckPlugin.php"
for a MediaWiki project.
Also, make sure that you have the following setting, or the plugin won't work:
'quick_mode' => false
You may also want to add SecurityCheck-LikelyFalsePositive
and
SecurityCheck-PHPSerializeInjection
to suppress_issue_types
(the latter
has a high rate of false positives).
Then run phan as you normally would:
$ vendor/bin/phan -d . --long-progress-bar
Standalone
- For MediaWiki core, add the following to composer.json:
"scripts": { "seccheck": "seccheck-mw" }
- For a MediaWiki extension/skin, add the following to composer.json:
"scripts": { "seccheck": "seccheck-mwext", "seccheck-fast": "seccheck-fast-mwext" }
- For a generic php project, add the following to composer.json:
"scripts": { "seccheck": "seccheck-generic" }
You can then run:
$ composer seccheck
to run the security check.
Note that false positives are disabled by default.
For MediaWiki extensions/skins, this assumes the extension/skin is installed in the
normal extensions
or skins
directory, and thus MediaWiki is in ../../
. If this is not
the case, then you need to specify the MW_INSTALL_PATH
environment variable.
This plugin also provides variants seccheck-fast-mwext
(doesn't analyze
MediaWiki core. May miss some stuff related to hooks) and seccheck-slow-mwext
(also analyzes vendor). seccheck-mwext
is several times slower than seccheck-fast-mwext
.
Note: Taint-check is bundled in https://github.com/wikimedia/mediawiki-tools-phan
version 0.10.2 and above, so you don't have to add it manually if you're already using
mediawiki-tools-phan
. For more information about Wikimedia's use of this plugin see
https://www.mediawiki.org/wiki/Phan-taint-check-plugin
Plugin output
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 involvedSecurityCheck-XSS
SecurityCheck-SQLInjection
SecurityCheck-ShellInjection
SecurityCheck-PHPSerializeInjection
- For when someone doesunserialize( $_GET['d'] );
This issue type seems to have a high false positive rate currently.SecurityCheck-CUSTOM1
- To allow people to have custom taint typesSecurityCheck-CUSTOM2
- dittoSecurityCheck-DoubleEscaped
- Detecting that HTML is being double escapedSecurityCheck-RCE
- Remote code execution, e.g.eval( $_GET['foo'] )
SecurityCheck-PathTraversal
- Path traversal, e.g.require $_GET['foo']
SecurityCheck-ReDoS
- Regular expression denial of service (ReDoS), e.g.preg_match( $_GET['foo'], 'foo')
SecurityCheck-OTHER
- Issues that don't fit another categorySecurityCheck-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 NORMAL 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.
How to avoid false positives
If you need to suppress a false positive, you can put @suppress NAME-OF-WARNING
in the docblock for a function/method. Alternatively, you can use other types of
suppression, like @phan-suppress-next-line
. See phan's readme for a complete
list.
The @param-taint
and @return-taint
(see "Customizing" section) are also very useful
with dealing with false positives.
Note that the plugin will report possible XSS vulnerabilities in CLI context. To avoid them,
you can suppress SecurityCheck-XSS
file-wide with @phan-file-suppress
in CLI scripts, or
for the whole application (using the suppress_issue_types
config option) if the application only
consists of CLI scripts. Alternatively, if all outputting happens from an internal function, you
can use @param-taint
as follows:
/** * @param-taint $stuffToPrint none */ public function printMyStuff( string $stuffToPrint ) { echo $stuffToPrint; }
When debugging security issues, you can use:
'@phan-debug-var-taintedness $varname';
this will emit a SecurityCheckDebugTaintedness issue containing the taintedness of $varname
at the line where the annotation is found. Note that you have to insert the annotation in a string
literal; comments will not work. See also phan's @phan-debug-var
annotation.
Notable limitations
General limitations
- 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
- The plugin won't recognize things that do custom escaping. If you have custom escaping methods, you must add annotations to its docblock so that the plugin can recognize it. See the Customizing section.
MediaWiki specific limitations
- 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.
- The plugin can only validate the fifth (
$options
) and sixth ($join_cond
) of MediaWiki'sIDatabase::select()
if its provided directly as an array literal, or directly returned as an array literal from agetQueryInfo()
method.
Customizing
The plugin supports being customized, by subclassing the SecurityCheckPlugin class. For a complex example of doing so, see MediaWikiSecurityCheckPlugin.
Sometimes you have methods in your codebase that alter the taint of a variable. For example, a custom html escaping function should clear the html taint bit. Similarly, sometimes phan-taint-check can get confused and you want to override the taint calculated for a specific function.
You can do this by adding a taint directive in a docblock comment. For example:
/** * My function description * * @param string $html the text to be escaped * @param-taint $html escapes_html */ function escapeHtml( $html ) { }
Taint directives are prefixed with either @param-taint $parametername
or @return-taint
. If there are multiple directives they can be separated by a comma. @param-taint
is used for either marking how taint is transmitted from the parameter to the methods return value, or when used with exec_
directives, to mark places where parameters are outputted/executed. @return-taint
is used to adjust the return value's taint regardless of the input parameters.
The type of directives include:
exec_$TYPE
- If a parameter is marked asexec_$TYPE
then feeding that parameter a value with$TYPE
taint will result in a warning triggered. Typically you would use this when a function that outputs or executes its parameterescapes_$TYPE
- Used for parameters where the function escapes and then returns the parameter. Soescapes_sql
would clear the sql taint bit, but leave other taint bits alone.onlysafefor_$TYPE
- For use in@return-taint
, marks the return type as safe for a specific$TYPE
but unsafe for the other types.$TYPE
- if just the type is specified in a parameter, it is bitwised AND with the input variable's taint. Normally you wouldn't want to do this, but can be useful when$TYPE
isnone
to specify that the parameter is not used to generate the return value. In an@return
this could be used to enumerate which taint flags the return value has, which is usually only useful when specified astainted
to say it has all flags.array_ok
- special purpose flag to say ignore tainted arguments if they are in an array.allow_override
- Special purpose flag to specify that that taint annotation should be overridden by phan-taint-check if it can detect a specific taint.
The value for $TYPE
can be one of htmlnoent
, html
, sql
, shell
, serialize
, custom1
, custom2
, code
, path
, regex
, misc
, sql_numkey
, escaped
, none
, tainted
, raw_param
. Most of these are taint categories, except:
htmlnoent
- likehtml
but disable double escaping detection that gets used withhtml
. Whenescapes_html
is specified, escaped automatically gets added to@return
, andexec_escaped
is added to@param
. Similarlyonlysafefor_html
is equivalent toonlysafefor_htmlnoent
, escaped.none
- Means no tainttainted
- Means all taint categories except special categories (equivalent toSecurityCheckPlugin::YES_TAINT
)escaped
- Is used to mean the value is already escaped (To track double escaping)sql_numkey
- Is fairly special purpose for MediaWiki. It ignores taint in arrays if they are for associative keys.raw_param
- To be used in conjunction with other taint types. Means that the parameter's value is considered raw, hence all escaping should have already taken place, because it's not meant to happen afterwards. It behaves as if the taint of the parameter would immediately be EXEC'ed
The default value for @param-taint
is tainted
if it's a string (or other dangerous type), and none
if it's something like an integer. The default value for @return-taint
is allow_override
(Which is equivalent to none unless something better can be autodetected).
Instead of annotating methods in your codebase, you can also customize phan-taint-check to have builtin knowledge of method taints. In addition you can extend the plugin to have fairly arbitrary behaviour.
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, escapes its argument and returns the escaped value would look like:
'htmlspecialchars' => [ ( self::YES_TAINT & ~self::HTML_TAINT ) | self::ESCAPED_EXEC_TAINT, 'overall' => self::ESCAPED, ];
Environment variables
The following environment variables affect the plugin. Normally you would not have to adjust these.
SECURITY_CHECK_EXT_PATH
- Path to directory containingextension.json
/skin.json
when in MediaWiki mode. If not set assumes the project root directory.SECCHECK_DEBUG
- File to output extra debug information (If running fromshell
,/dev/stderr
is convenient)