Security checklist for developers

From MediaWiki.org
Jump to: navigation, search

This document is provided as a supplement to Security for developers. This is a list of common development tasks, and the security measures that need to be taken.

Security checklist[edit]

If you are working with ... have you ...

Cookies[edit]

  • reduced reviewer anxiety by using $wgRequest instead of $_COOKIE?
    • fetched cookies using $wgRequest->getCookie(...)?
    • set cookies using $wgRequest->response()->setcookie(...)?
# Attempt to fetch the UserID cookie value. Note: The 
# value returned isn't trusted and is forced to be an int.
$sId = intval( $wgRequest->getCookie( 'UserID' ) );

Dynamic code generation[edit]

Avoid using functions like eval() and create_function(), as well as the /e pattern modifier for preg_replace(). While powerful and convenient, these features are inherently insecure:[1][2]

  • it's easier to put arbitrary strings into text processed by a regular expressions, which – when combined with the /e pattern modifier – can lead to code injection attacks.
  • it is harder to read and maintain code that is part of a string.
  • static analysis tools won't catch warnings and errors in the code.
  • opcode caches (like APC) can't cache code mixed into strings.
  • create_function() sometimes has garbage-collection issues.
  • A loop which has a create_function() inside will create a new function on each iteration.

Sometimes you really do need these features (obviously eval.php needs to run eval() ;) but in most cases, we'd rather see the function broken out and referred as a callback.

Inline lambda functions will make it easier to make your callback inline while retaining the benefits of code that's written in native syntax instead of strings.

  • Anything external that is used in part of regex should be escaped with preg_quote( $externalStr, $delimiter ). It puts a backslash in front of every character that is part of the regular expression syntax, and escapes also the delimiter given as second parameter:
$str = preg_replace( "!" . preg_quote( $externalStr, '!' ) . "!", $replacement, $str );

External programs[edit]

  • executed the program via Shell::command() from namespace MediaWiki\Shell?
  • quoted all arguments to external programs using the above's secure parameter passing facilities (which is basically everything except for unsafeParams())?
// Automatically escape any naughty characters
$result = Shell::command( $cmd, '--version' )
    ->params( 'some', 'extra', 'parameters' )
    ->execute();

Note that old wfShellExec()/wfEscapeShellArg() are not recommended because they make it easier for developers to miss escaping a parameter.

Forms[edit]

GET data[edit]

  • reduced reviewer anxiety by using $wgRequest instead of $_GET?
# Check if the action parameter is set to 'purge'
if ( $wgRequest->getVal( 'action' ) == 'purge' ) {
    ...

Global variables[edit]

# ensure that the script can't be executed outside of MediaWiki
if ( !defined( 'MEDIAWIKI' ) ) {
    die( 'Not a valid entry point.' );
}

Output (API, CSS, JavaScript, HTML, XML, etc.)[edit]

Any content that MediaWiki generates can be a vector for XSS attacks.

  • used the Html and Xml helper classes?
# rawElement() escapes all attribute values 
# (which, in this case, is provided by $myClass)
echo Html::rawElement( 'p', array( 'class' => $myClass  ) );
  • reduced reviewer anxiety by using ResourceLoader to deliver CSS and JavaScript resources?

User provided CSS[edit]

User provided CSS (Say for use in a style attribute) needs to be sanitized to prevent XSS, as well as to disallow insertion of tracking images (via background-image), etc

  • Use the Sanitizer::checkCss method for any css received from a user, possibly along with the Html class.
# let $CSSFromUser be the user's CSS.
echo Html::rawElement( 'p', array( 'style' => Sanitizer::checkCss( $CSSFromUser ) ) );
  • For CSS provided by the extension (and not the user), this is not needed (and will remove some valid things like background-image:). However, extension provided CSS should go in stylesheets loaded by ResourceLoader, and not in style attributes.

POST data[edit]

  • reduced reviewer anxiety by using $wgRequest instead of $_POST
  • Always validate that any POST data received is what you expect it to be
# Check if the action parameter is set to 'render'
if ( $wgRequest->getVal( 'action' ) == 'render' ) {
    ...

Query strings[edit]

Sessions[edit]

Any user input: no isset![edit]

Any isset() is frowned upon. Use strong comparison like === or !==. An example from SimpleAntiSpam:

	if ( !( $_POST['name'] === "" && $_POST[$bottrop_field_name] === $bottrop_field_value ) ) {
		return( "Stuff" );
	}

See also Register globals#Sanitize custom global variables before use and Manual:Coding conventions/PHP#Pitfalls.

Reviewer anxiety[edit]

  • Clearly added comments to explain unexpected or odd parts of your code?
# $wgRequest isn't yet available.  Forced to use $_GET instead. 
if ( $_GET['setupTestSuite'] !== null ) {
 	$setupTestSuiteName = $_GET['setupTestSuite'];
 	...

SQL queries[edit]


See also[edit]

References[edit]

Coding conventionsManual:Coding conventions
General All languagesManual:Coding conventions#Code structure · Development policyDevelopment policy · Security for developersSecurity for developers · Pre-commit checklistManual:Pre-commit checklist · Performance guidelinesPerformance guidelines (draft) · Style guideDesign/Living style guide · Accessibility guide for developersAccessibility guide for developers (draft) · Best practices for extensionsBest practices for extensions
PHP Code conventionsManual:Coding conventions/PHP · PHPUnit test conventionsManual:PHP unit testing/Writing unit tests#Test_conventions · Security checklist for developersSecurity checklist for developers
JavaScript Code conventionsManual:Coding conventions/JavaScript · Learning JavaScriptLearning JavaScript
CSS Code conventionsManual:Coding conventions/CSS
Database Code conventionsManual:Coding conventions/Database · Database policyDevelopment policy#Database policy
Python Code conventionsManual:Coding conventions/Python
Ruby Code conventionsManual:Coding conventions/Ruby
Selenium/Cucumber Code conventionsManual:Coding conventions/Selenium
Java Code conventionsManual:Coding conventions/Java
API client code Standards for API client librariesAPI:Client code/Gold standard