Skip to Content

Newsfeeds

godel.com.au: Ideas to help keep your Drupal project secure against the OWASP Top 10

Planet Drupal - 10 July 2014 - 10:24pm
Fri July 11, 2014 Ideas to help keep your Drupal project secure against the OWASP Top 10

I'm sure you've heard the phrase "Security is a process, not a product" before, or something along those lines. Drupal has a pretty good track record as far as Web-based CMS security goes, and there's a dedicated team of experts looking after Core and Contrib, but it's no secret that the most common vulnerabilities are those we accidentally introduce ourselves during our day-to-day tweaking (Drupal.org claims over 90%). The Drupal Security White Paper shows pretty clearly that the further we get away from the heavily peer-reviewed Core codebase, the more likely we are to unwittingly introduce something that's dangerous to our data.

In the case of what processes we can implement as individuals, self education and reflection is always important, so I figured that the present is as good a time as any to run through the OWASP Top 10 and present a few practical ideas and Drupal "Best Practices" to help mitigate the risks we inevitably expose ourselves to. I've added some quotes from the white paper to each section to put things into perspective.

#1 - Injection

Drupal contains a robust object-oriented database API that makes it difficult for developers to unknowingly create injection holes by automatically sanitizing query parameters and enforcing an interface. Drupal's file system interaction layer limits where files can be written and alters dangerous file extensions that the server could potentially execute.

In Drupal, injection attacks are generally going to be enabled by allowing unsafe data to sneak into SQL queries. In Drupal 7 we have two syntaxes for working with the database API, the classic Drupal 6 style db_query("SELECT * FROM foo f WHERE bar = 'baz'"); and the dynamic DBTNG style db_select('foo', 'f')->condition('bar', 'baz' , '=');.

For both syntaxes, there are two things I think are worth mentioning:

Make use of PHP's PDO and Prepared Statements

Drupal's database API wraps PHP's own PDO, which supports something called Prepared Statements that magically guarantee you won't suffer from an injection attack provided you actually use the API correctly.

So, if you're wondering how to use the API correctly, let's split all SQL queries we might want to use into two categories, those that could be considered hardcoded strings, and those that have one or more variable components. The examples above would be "hard coded", an example of something with variable components might look like this:

// Get the type of the node with the ID of the second URL component.
// WARNING: Don't actually do this! it is NOT secure.
$id = arg(1);
$query = db_query("SELECT type FROM node n WHERE n.nid = " . $id);

The issue here is that, unlike our hardcoded query, the value of $id can be set to anything at all by the end-user, most notably including strings that resemble fragments of SQL queries. We really don't want the end-user to discover one day that they can start executing their own queries against our database by carefully crafting URL components to exploit our PHP logic.

Here the fix is to either use PDO placeholders using Drupal's classic syntax, or leverage Drupal's internal query builder with the dynamic syntax. The following two examples (one for each syntax) are secure versions of the previous insecure example:

Drupal 6 syntax:

// Get the type of the node with the ID of the second URL component. // The important part here is that the second argument passed to db_query() // is an array of substitutions that will be sanitized internally. $id = arg(1); $query = db_query("SELECT type FROM node n WHERE n.nid = :nid", array(':nid' => $id));

Drupal 7 syntax:

// Get the type of the node with the ID of the second URL component. // The important part here is that we use the dynamic query builder to handle // the unsafe $id variable. $id = arg(1); $select = db_select('node', 'n') ->condition('n.nid', $id, '=') ->fields('n', array('type'));

The question now is, how do we know when a variable is "unsafe"?

The answer should really be, treat all variables as though they are unsafe by default and stop guessing. Be in the habit of using the more secure syntax by default, even when it is not strictly neccessary. This both reduces the risk of some edge-case creeping up on you that you hadn't considered, and makes your code more portable/standardised at the same time.

Make use of whitelists for tables/fields/operators

I think maybe this one is less obvious to many people than making use of PDO, but it is also important to consider.

As mentioned in a bunch of core issues, and elsewhere (read the comments) Drupal doesn't actually escape/sanitize everything that you pass the database API. The reason that those issues are still open and not flagged as "critical fix me NOW ZOMG" (In part), is because certain parts of our SQL queries are fundamentally unsafe to use user data for, even if they were being escaped by the system.

Imagine a query (presumably hooked up to a radio button or select element) like the following:

// Return the current user's username or email address to be displayed. // WARNING: Don't actually do this! it is NOT secure. global $user; $field_name = $form_state['values']['field']; $select = db_select('users', 'u') ->condition('u.uid', $user->uid, '=') ->fields('u', array($field_name));

In this case, the end-user can hack up the form you presented them however they like and submit something for the field form value like "password" to get a dump of what is stored in the password field for the current user account. It's not hard to imagine other, even more damaging scenarios.

Since Drupal knows that it is fundamentally insecure to be directly using user data for some things (e.g. table names, fields, operators in condition() and orderBy()) and assumes that you'll definitely be using whitelisted values only in these cases. There may well be a de-prioritised, outstanding Core bug that means your variables are not being sanitised/escaped at all, despite the fact that you're using the query builder. This means that, as well as the end-user getting at your user's passwords when you thought they could only see email addresses, they may also have a way to directly attempt an injection attack on your database.

The fix here is, make use of whitelists wherever you're dealing with variables for finite, pre-determined lists of valid options (like field names). For example, the above could be rewritten to be more secure like this:

// Return the current user's username or email address to be displayed. global $user; // Return the username by default. $field_name = 'username'; // Ony allow field names in our whitelist. $allowed_fields = array('username', 'mail'); if (in_array($allowed_fields, $form_state['values']['field'])) { $field_name = $form_state['values']['field']; } $select = db_select('users', 'u') ->condition('u.uid', $user->uid, '=') ->fields('u', array($field_name)); #2 - Broken authentication and session management

User accounts and authentication are managed by Drupal core. Authentication cookies and a user's name, ID, and password are managed on the server to prevent a user from easily escalating authorization. User passwords are salted and hashed using an algorithm based on the Portable PHP Password Hashing Framework and existing sessions are destroyed upon login and logout.

Drupal handles this well "out of the box" and most developers will be rarely doing anything that would break it. According to the white paper, there are very few Security Advisories being issued in this category, so I won't dwell on this.

#3 - Cross site scripting (XSS)

Drupal has a strong system for filtering user-generated content on display. Untrusted user's content is filtered to remove dangerous elements by default. For developers, Drupal has at least eight API functions for filtering output to prevent XSS attacks. When identified, common developer errors that lead to XSS vulnerabilities are mitigated by building safer defaults. For example, a page title function in Drupal 6 is the source of many XSS holes due to a lack of proper escaping. In Drupal 7 this function escapes output by default.

Hmmm, I think the words "by default" mean something different to the authors of the white paper than they do to me. If you do anything in the Drupal theme layer, and you don't explicitly, manually sanitise every single one of your variables as you render them, you have XSS holes in your theme.

In Drupal 8, there's a critical issue to try and get automatic escaping of markup into core by using the Twig rendering system which should dramatically increase security (in line with my intuitive definition of "by default") and improve performance as a nice bonus. That issue attempts to summarise the situation:

No-one can write XSS safe code. Nor core contributors, nor contrib developers, no-one.

and from a duplicate issue:

100% of all custom themes are insecure (the rest is rounding error). The reason is often that developers forget to escape their data in the templates.

By the white paper's own reported statistics, over 50% of reported security vulnerabilities in Drupal core and contributed community code are XSS issues.

In light of all this, between now and when Drupal 8 auto-escaping is released, even if you learn nothing else about Drupal security, you'll want to become familiar with a few of the API functions provided for string sanitisation.

For reference, the full list on api.drupal.org is:

Read up on all of the sanitisation functions, but the most commonly used (for general theming) are probably:

check_plain()

Use this when you have a string that is NOT supposed to be HTML, but you want to display it in an HTML context. It's really just a wrapper around htmlspecialchars(). Any characters in your non-HTML string that might be interpreted as HTML by the browser are converted into HTML entities so that they render as you would expect them to in a non-HTML context.

This means that your string that looks like X < Y & Y > Z becomes X &lt; Y &amp; Y &gt; Z.

A couple of things to keep in mind with this function:

  1. It's possible to accidentally double encode characters so that &amp; becomes &amp;amp;, which looks really bad when rendered. It is NOT a good idea to run check_plain() over everything without thinking, in case you run it on a string that has already had check_plain() run on it earlier.
  2. While check_plain() is an important function for security, "defusing" characters that could be interpreted as HTML, it's primary usage is for encoding/decoding characters, NOT guaranteeing that strings are "HTML safe" by stripping dangerous markup. For example, it's possible that a substring of a string that was previously run through check_plain() will have its encoding corrupted if an HTML entity is truncated before the closing ;.
check_markup()

Use this to apply input filters to a string that is intended to be used as HTML. In this case, we DO want HTML to be interpreted by the browser, but we also want to be careful not to open ourselves up to <script> elements or similar being executed maliciously. In the simplest usage, this function takes two arguments - the string and the machine name of the filter format to apply.

Filter formats themselves are an important part of core and too detailed a subject for me to go into here. Suffice to say that the "out of the box" formats set the tone for most custom filters created by developers:

  1. plain_text - Behaves as check_plain() does.
  2. filtered_html - Will strip any (customisable) HTML tags considered dangerous and leave the rest intact
  3. full_html - Will render any and all HTML tags in the string, even if they might be dangerous. (ab)Using this causes developers to grumble.

Avoid using the PHP input filter completely if you care at all about mitigating security risks.

t() and/or format_string()

Use this in situations where you want to perform simple string substitutions with variables (that might come from end-users data) and, in the case of t() also allow for the string to be translatable into other languages.

Since t() wraps format_string(), I encourage you to use the former as they behave the same way and you get the translatability "for free".

The advantage of using placeholders in strings, rather than concatenating variables, is twofold, format_string() will automatically apply check_plain() to substitutions that do NOT begin with ! and it makes the strings easier to contextualise for translaters.

// Here the translators can see a whole sentence and use the placeholders as clues. // We don't have to call check_plain() repeatedly because we're using @placeholder syntax. t('Yesterday I went to @place and brought home a @thing', array( '@place` => 'the park', '@thing' => 'frisbee', )); // Here translators only see two disjointed strings separately: // 'Yesterday I went to' and 'and brought home a'. t('Yesterday I went to ' . check_plain($place) . 'and brought home a ' . check_plain($thing)); #4 - Insecure direct object references

Drupal often provides direct object reference, such as unique numeric identifiers of user accounts or content available in the URL or form fields. While these identifiers disclose direct system information, Drupal's rich permissions and access control system prevent unauthorized requests. Methods for obfuscation are available through configuration and community-contributed code. Further, validation and protection against semantic forgery attacks is implemented in Drupal core via the Form API.

For Drupal, this largely comes down to making sure you have your server environment and Drupal site configured in a way that makes sense for your project, in the context of Drupal, I'm going to tie this into...

#5 - Security misconfiguration

Many critical risks, such as access to administrative site controls, text formats, and private information are restricted to a single admin account by default. Identified design inefficiencies that lead to misconfiguration are corrected often through usability testing and fixes are recommended for inclusion to core. Documentation of best practices for secure configuration and site building are provided for free on drupal.org and there are several contributed projects that conduct automated security review or implement further secure configurations.

In my experience, the two fastest ways to end up with hundreds of spambot accounts (or worse) on your account are:

  1. Servers configured in a way that is not Drupal friendly, lazily or by someone who isn't an experienced sysadmin.
  2. Developers building functionality as Drupal user 1 and never logging out or logging in as a user with a different role to test it.

For the first point. If you are not a sysadmin, you don't directly employ a sysadmin, or you don't have a friend who is a sysadmin who owes you a series of large favours, it is going to be much, much cheaper and less headache inducing to sign up with Acquia or Pantheon than to mess around with a shared hosting provider or to try and configure and maintain your own VM.

For the second point, the Drupal permissions grid is huge and tedious to manage as it involves a lot of user switching but there are a few things you can do for yourself to make maintaining complex permission sets easier.

Implement a simple, repeatable testing plan

In the Drupal community, SimpleTest and Behat are very popular testing tools. Both support simple assertions for the HTTP response code and the existence/absences of HTML elements on a rendered page. In fact, automated tests to check that a given user has permission to do something or visit certain pages are some of the easiest to write and can help you avoid those awkward conversations where we have to explain to someone that there was a regression in the site's fundamental security configuration.

Even if you don't have the time or resources to set up automated tests for a project, make step-by-step notes of critical functionality and user roles that can be tested as part of QA each release.

If you're using drush, you can generate a one time login for any user in the system with uli and their username:

$ drush @mysite.alias uli 'John Smith'

Have a deployment plan

People who don't know how to use the Drupal permissions grid just love to make changes to it, then immediately forget what changes they made, or tell anyone that they made the changes. That is, if you let them.

At the very minimum, you should be exporting your permissions into Features so that, should anything go wrong, you have some kind of "oh shit" button to get things back to working order, fast (without rolling back your database).

The Features approach is a little limiting for Permissions management though:

  • The permissions aren't actually locked. It's relatively easy for a permission to be changed on production and it won't be restored to the correct value until the Feature is next reverted.
  • It's not easy to toggle permissions on/off for different development environments. For most permissions you aren't going to want to do this anyway, but for those that you do, you really don't want the wrong permissions in the wrong environment.
  • There are a very large number of Permissions, and exporting them all to a Feature quickly bloats the Feature code and UI.
  • Adding a permission to a Feature artificially introduces a dependency of the Feature on that module.
  • Exporting permissions to Features is a tedious and error-prone manual process.
  • Permissions exported in Features are not very portable between projects.
  • The generated code in Features is difficult to read over quickly.
  • It's relatively difficult to manage permissions across multiple domains if you have a multisite setup.

There are various contrib modules that aim to make permissions management more secure and development friendly, to name a few:

Additionally, you might want to consider making permissions management part of your deploy/build scripts if you use such things to manage your environments.

#6 - Sensitive data exposure

Account passwords are salted and repeatedly hashed based on the Portable PHP Password Hashing Framework. Available Drupal community-contributed code offer solutions to encrypt sensitive data at rest or in transit.

This depends a lot on what your employer/client/government considers "sensitive" data. Where I live, in Australia, the government has published a convenient set of guidelides around sensitive and private data that can be used to gauge the level of risk if data were to be exposed. If you don't have any in your site, well that's great - although I still recommend implementing hook_menu() to unset the default node menu path if you haven't already, as it shows every node in the site by default, which can be awkward.

If you're working with Ubercart or Commerce, please, please triple check whether your payment gateway integration module sends user's credit card data through the Drupal Form API, to your server, before sending it to the payment gateway provider. This goes double (so sextuple check) for anything that claims to store credit card data for later use, even if it is "tokenized". If sensitive credit card (or similar) data touches your server even for a moment, you might be exposing yourself to fines if you haven't taken appropriate steps (PCI-DSS) to ensure encryption and security.

If you work with Drush, and multiple environments, you might want to consider implementing hook_drush_sql_sync_sanitize() for your project. This allows you to scrub out sensitive data from your production database as you pull it down, so that you don't accidently lose it to someone shady along with your laptop or not-so-trustworthy contractor. You don't need to use drush for this, there are lots of ways, including a simple shell script with SQL queries, to setup a decent database scrubbing script. Acquia have some documented examples of how to achieve this too, if you're interested.

#7 - Missing function level access control

Function level access in Drupal is protected by a powerful permissions-based system which checks for proper authorization before the action is taken. For the case of URL access, access checking is tightly integrated into the entire menu-rendering and routing system which means that visibility of navigation links and pages are protected by the same system that handles incoming requests.

Yes, menu-rendering uses access checking to ensure that navigation links are protected, but links generated by l() or #type => 'link' render arrays do not automatically run access checks. Function (and URL) level access control is very easy to implement in Drupal using hook_permission(), hook_menu() and user_access(). Creating new, custom permissions is very straightforward but it's also very easy to get wrong in subtle ways (especially if you're writing some new function that wraps another function with existing access checks) - as I mentioned above, make sure you have a solid testing plan in place for your new permissions scheme before you deploy it!

#8 - Cross-Site request forgery

Drupal validates user intention in actions using industry standard techniques. Typical actions with side- effects (such as actions that delete database objects) are generally conducted with the HTTP POST method. Drupal's Form API implements unique form tokens to protect against CSRF in POST requests. Less important actions can leverage the one-time token generation and validation functions of the Form API while still using an HTTP GET request.

Reviewing the security white paper, we can see that there's a relatively large incidence of contributed, non-core code that is not correctly implementing the Drupal Form API, leading to CSRF exploits.

Avoiding CSRF exploits in Drupal is relatively easy if you follow three simple rules:

  1. Whenever you expose an action to an end-user that modifies data in your site, implement it using a form (POST) rather than a link + menu callback (GET).
  2. Never try to build an HTML form manually, always use renderable form arrays and the Form API.
  3. Always use form validate/submit handlers and $form_state to access user submitted values rather than trying to extract information from $_POST directly.

If you need to quickly convert some GET based code you suspect might be at-risk to POST requests, the Drupal documentation suggests converting the page callback to a simple confirmation form using the confirm_form() core function as a base.

Another option, is to put a token in the URL that is unique for each page load and can't be easily forged, much like the Flag module does. Keep in mind though that this approach might not behave well in conjunction with cached markup containing links to URLs that rely on tokens like this.

#9 - Using components with known vulnerabilities

Included libraries and frameworks (of which there are few) in Drupal core are system-level, unsophisticated, and of low risk to full server or application compromise.

Unfortunately, this happens very, very frequently in Drupal as soon as you start counting contributed modules as "components". Ironically, the frequency of known security vulnerabilities showing up in installed modules is due in a large part to the security team doing a good job of reviewing and flagging community code. The difficult part here is being proactive about regularly deploying new module versions as security fixes are released.

It's a shame that the Droptor service was discontinued, as it provided a central dashboard for reviewing available security updates across multiple sites. Acquia and Pantheon both provide similar dashboards for sites hosted on their servers.

It's also a shame that the core Update module that "dials out" to Drupal.org to get the status of installed module versions is somewhat poorly optimised. It's very easy to configure a site (such as when using "poor man's cron") so that 20+ second page loads are experienced intermittently when using Update. If performance is important, I recommend enforcing the Update module to be enabled in a review environment, but not production or local development where slow page load times could become an issue.

There's little else to say here. Keeping your modules up-to-date is simply a matter of organisation/process and diligence and has little to do with development skills or further learning.

#10 - Unwanted redirects and forwards

Internal page redirects cannot be used to circumvent Drupal's integrated menu and access control system. Drupal protects against automatic redirection to off-site URLs which could be used in a phishing attack.

Drupal provides the drupal_goto() function for handling redirections correctly. This function will ignore absolute URLs in GET requests, but otherwise will always respect the destination parameter in the URL. This allows us to safely redirect/forward users by constructing our URLs correctly (with drupal_get_destination()), without opening ourselves up to phishing attacks.

The contrib External Links module also provides visual feedback for users, protecting them against unexpected offsite redirections, showing an icon on any link that will take the user to a new domain. The External Links Extra module takes this one step further and allows for a popup warning box, or even a dedicated intermediate page, to make it as clear as possible to users that they are about to be redirected offsite.

David Meister Director & Lead developer Using style-linking for sane @font-face declarations in a post-IE8 world Mon June 30, 2014 Say no to fake browser bold and fake browser italic, messy @font-face syntax and super-legacy browser bug support with the neat, logical style-linking @font-face syntax. Tags planet drupal
Categories: Drupal

Excerpts from An Architectural Approach to Level Design - by Christopher Totten

Gamasutra.com Blogs - 10 July 2014 - 10:17pm
In this post, I provide a few excerpts from my new book, An Architectural Approach to Level Design, which was recently published by CRC Press.
Categories: Game Theory & Design
Syndicate content


Google+
about seo