Last updated January 11, 2014. Created by ronaldmulero on May 7, 2006.
Edited by mlhess, pwolanin, Heine, jhodgdon. Log in to edit this page.

Know about a security issue? Please alert the security team.

Whether you are writing a PHP snippet or an entire module, it is important to keep your code secure.

Use check functions on output to prevent cross site scripting attacks

No piece of user-submitted content should ever be placed as-is into HTML.

  • Use check_plain for plain text.
  • Use filter_xss for markup containing text, or if the context is entered by an admin and should allow most markup, use filter_xss_admin. While it can also sanitize text, it's almost never correct to use check_markup in a theme or module except in the context of something like a text area with an associated text format.
  • Use the t() function with @ or % placeholders to construct safe, translatable strings.

See how to handle text in a secure fashion for more details.

Use the database abstraction layer to avoid SQL injection attacks

Use the database layer correctly. For example, never concatenate data directly into SQL queries, like this:

<?php
db_query
('SELECT foo FROM {table} t WHERE t.name = '. $_GET['user']);
?>

Instead, use proper argument substitution with db_query:

For Drupal 6 and before

<?php
db_query
("SELECT foo FROM {table} t WHERE t.name = '%s' ", $_GET['user']);
?>

If you have to accommodate a variable number of arguments in your SQL, create an array of placeholders. Do NOT do this:

<?php
db_query
("SELECT t.s FROM {table} t WHERE t.field IN (%s)", $from_user);
?>

Instead, do this:

<?php
$placeholders
= implode(',', array_fill(0, count($from_user), "%d"));

db_query("SELECT t.s FROM {table} t WHERE t.field IN ($placeholders)", $from_user);
?>

For Drupal 7+

The DB layer works on top of PHP PDO and uses an array of named placholders:

<?php
db_query
("SELECT foo FROM {table} t WHERE t.name = :name", array(':name' => $_GET['user']));
?>

For a variable number of argument, use an array of arguments or use db_select():

<?php
db_query
("SELECT t.s FROM {table} t WHERE t.field IN (:users)",  array(':users' => $from_user));
?>

OR

<?php
$result
= db_select('table', 't')
    ->
fields('t', array('s'))
    ->
condition('t.field', $from_user, 'IN')
    ->
execute();
?>

Use db_rewrite_sql to respect node access restrictions.

Most SQL statements which refer to nodes or the {node} table should be wrapped in a db_rewrite_sql() function call:

<?php
$result
= db_query(db_rewrite_sql("SELECT n.nid, n.title FROM {node} n"));
?>

Drupal's node access mechanism requires such calls. Without them, visitors may gain access to nodes that they don't have permission to view.

Looking for support? Visit the Drupal.org forums, or join #drupal-support in IRC.

Comments

Heine Deelstra, security team leader for Drupal, wrote an article on the development mailing list some time back in December 2006.

Since he has no time to refine it in a page, I am posting it below as a comment, so someone can take it and merge it with the other pages.

=========================
Porting a module is an excellent opportunity to keep an eye out for security problems (evidence: http://drupal.org/node/103943).

Here's a quick security reminder regarding input (user-supplied data).

Code samples are only included to make a point, do not hold them against me.

Escape or filter text before display
====================================
If you do not escape or filter text before display, you enable a user to insert arbitrary HTML and scripts into pages. This type of attack is know as cross site scripting aka XSS.

For more information, see: http://drupal.org/node/28984

Escape / filter:
- check_plain or theme('placeholder') for plain text
- check_markup or filter_xss for markup containing text

Drupal 5 brought a (brilliant IMO) change to t():

Depending on the placeholder's sigil, it is passed through theme('placeholder') (%) or check_plain (@) automatically.

t('I escape %user_data', array('%user_data' => $data)); // I escape <em>user_data</em> (safe)
t('I escape @user_data', array('@user_data' => $data)); // I escape user_data (safe)
t('I don't escape !user_data', array('!user_data' => $data));// XSS vulnerability

Sometimes the difficult part is to know whether a function requires escaped text
or does the escaping itself:

* Automatically escapes:

l() - Escapes text and attributes unless you pass TRUE for the $html parameter.
In that case only attributes are check_plained.

- menu items & breadcrumbs
- block descriptions (*not* titles)
- theme('username')

* Common pitfalls:
watchdog - you have to provide a safe $message
drupal_set_title - you have to provide a safe $title
Form elements - #description and #title of form_elements need to be safe
Form elements - #value of #type markup and item need to be safe. Note that the
default form element #type is markup!

Examples:

drupal_set_title($node->title); // XSS vulnerability
drupal_set_title(check_plain($node->title));

watchdog('content', t("Deleted !title", array('!title' => $node->title)); // XSS
watchdog('content', t("Deleted %title", array('%title' => $node->title)); // or @

$form['unsafe'] = array('#value' => $user->name); //XSS
$form['safe'] = array('#value' => check_plain($user->name));
or
$form['safe'] = array('#value' => theme('username', $user));

$form['bad'] = array(
'#type' => 'textfield';
'#default_value' => check_plain($u_supplied),  // bad: escaped twice
'#description' => t("Old data: !data", array('!data' => $u_supplied)), // XSS
);
$form['good'] = array(
'#type' => 'textfield';
'#default_value' => $u_supplied,
'#description' => t("Old data: @data", array('@data' => $u_supplied)),
);

Database (ab)use
====================================

See also http://drupal.org/node/101496

Summary: Whatever you do, *never* use user supplied data directly in a query. Use db_query as it was meant to be used: data goes in arguments. And make sure to enclose %s in single quotes: '%s'.

BAD, BAD, BAAAD:

1. db_query("SELECT t.s FROM {table} t WHERE t.field = $from_user");
2. db_query("SELECT t.s FROM {table} t WHERE t.field = %s", $from_user);

Good:
db_query("SELECT t.s FROM {table} t WHERE t.field = '%s'", $from_user);

BAD ($from_user is an array of numbers):

db_query("SELECT t.s FROM {table} t WHERE t.field IN (%s)", $from_user);

Good:

$placeholders = implode(',', array_fill(0, count($from_user), "%d"));

db_query("SELECT t.s FROM {table} t WHERE t.field IN ($placeholders)", $from_user);

Anything else: bad. Why is this bad? Suppose you check the data that you stuff in to a query (e.g. 'all numeric'). Or, you know a user name is validated to contain only certain 'safe' characters.

The problem is that you open the possibility that a code path develops where unfiltered content ends up at your query. In that case you're toast.

This code path can arise because you 1) accidentally remove validation in future development, 2) a user adds a module that is more lenient in the data it accepts or 3) you add a bunch of code that skips validation (recent xTracker problem).

Writing secure code
====================================
If you want to read more about security, I suggest that you start at "Input the root of all evil" http://drupal.org/node/101495 in the "Writing secure code" http://drupal.org/node/62304 pages.

I hope to expand and improve those pages in the future, but some of them are already useful.

You found a vulnerability?
====================================
What to do when you find a vulnerability in your module? Simple; write us at security@drupal.org.

Thank you for listening,

Heine Deelstra

--
Drupal development and customization: 2bits.com
Personal: Baheyeldin.com

--
Drupal performance tuning and optimization, hosting, development, and consulting: 2bits.com, Inc. and Twitter at: @2bits
Personal blog: Ba

I looked at the slideshare posted above. Slide #22 is a diagram showing when to use

  • check_url
  • check_plain
  • check_markup
  • filter_xss

That diagram did more in 1 second to explain when to use these functions than half an hour of reading handbook and api text. Could that slide be used either here (in the "Use check functions on output to prevent cross site scripting attacks" section), and/or on the Handle user input with care page?

As pointed out to me by sreynen in my project application:

<?php
$placeholders
= implode(',', array_fill(0, count($from_user), "%d"));
db_query("SELECT t.s FROM {table} t WHERE t.field IN ($placeholders)", $from_user);
?>

Should (for Drupal 6.x) use db_placeholders

<?php
$placeholders
= db_placeholders($from_user, 'int');
db_query("SELECT t.s FROM {table} t WHERE t.field IN ($placeholders)", $from_user);
?>

Just for clarity, "Use check_markup or filter_xss for markup containing text" should probably say "markup-containing text."

This is a great article, and I learned a lot of specifically helpful information. Anyone writing modules needs to review this information carefully.

Patty Zevallos
web architect/content producer
www.pbzproductions.com

The IN clause example shown above that uses "implode" to create $placeholders will not work in Drupal 7; instead, you should do this:

<?php
   db_query
("SELECT t.s FROM {table} t WHERE t.field IN (:ids)", array(':ids' => $from_user));
?>

See the "Placeholder arrays" section of http://drupal.org/node/310072 for details.