Heine

  • home
  • drupal
  • drupal core commits
  • about
Home › Drupal

Access denied - Are you sure?

Heine — Fri, 21/03/2008 - 05:23

It is surely not my intend to discuss each and every security announcement, but the recent Live announcement (SA-2008-021) deserves to get some attention as this particular drupal_access_denied & drupal_not_found pitfall affects you too, especially when you are converting "arrow code" to use guard clauses.

// Arrow code.
// This drupal_access_denied / drupal_not_found pattern is common in Drupal modules.
function page_callback() {
  if (condition) {
    if (another_condition) {
      if (yet_another_condition) {
        return do_something();
      }
    }
  }
  drupal_access_denied();
}

// Guard clauses with a security vulnerability ahead.
function page_callback() {
  if (!condition) {
    drupal_access_denied();
  }
  if (!another_condition) {
    drupal_access_denied();
  }
  if (!yet_another_condition) {
    drupal_access_denied();
  }

  return do_something();
}

The pitfall: drupal_access_denied emits the access denied page, but does not end the request. This means do_something() is still executed, with potential security vulnerabilities as a result. When you actually test an access denied scenario and do_something returns a string, you'll notice you get a double page rendered; one part contains the access denied page, the other the output of do_something.

The remedy is easy, use return drupal_access_denied(); instead of drupal_access_denied();

// Guard clauses (these can often be collapsed into one if()).
function page_callback() {
  if (!condition) {
    return drupal_access_denied();
  }
  if (!another_condition) {
    return drupal_access_denied();
  }
  if (!yet_another_condition) {
    return drupal_access_denied();
  }

  return do_something();
}

Average: 4.9 (11 votes)
  • Drupal
  • Pitfalls
  • Planet Drupal
  • Security
  • Login or register to post comments

Hi, I just did a search in a

peach - all drupal themes (not verified) — Fri, 21/03/2008 - 10:26

Hi, I just did a search in a folder with a bunch of drupal modules;

*.module; "return drupal_access_denied" : found 4 files

* .module; " drupal_access_denied" (note 2 spaces before function) : found 21 files

Does that mean all the modules in the second group are insecure? or possibly insecure?

  • Login or register to post comments

maybe

greggles (not verified) — Fri, 21/03/2008 - 14:09

It depends what comes on the following lines.

if it just does

if (something) {
  drupal_access_denied();
  something_else_important();
  return;
}
other_stuff();

Then the other_stuff is protected by the return on the following line.

  • Login or register to post comments

I want to know how Heine has

xamox (not verified) — Sun, 23/03/2008 - 03:06

I want to know how Heine has become such a security ninja. He seems to be the master at finding these security bugs. Props to you man.

  • Login or register to post comments

excellent

Pedro Pablo (not verified) — Thu, 17/07/2008 - 00:00

it is so comfortable to know people like you are watching out to protect us from the bad guys... :-)
Very good discovery as it will enhance many modules' security

  • Login or register to post comments

In a lot of cases you won't

Xano — Sat, 20/09/2008 - 00:27

In a lot of cases you won't be able to end the request by returning drupal_access_denied() or drupal_not_found(), because the function these calls are made in are no direct menu callback. The solution is to simply exit; right after calling one of the aforementioned functions.

  • Login or register to post comments

Recent posts

  • Unserializing user-supplied data, a bad idea
  • Planet Drupal past and current
  • Help! - Cannot access a global variable.
  • Why is my module's update hook not listed on update.php's selection form?
  • How do I add a class to a link generated with l()
more

Security reviews

  • Afraid custom code makes your site vulnerable?
  • You don't really trust that module you just downloaded from Drupal.org?

Sleep better after a security review.

Tags

Captcha CSRF Drupal embed Input Format modx OpenID Performance Planet Drupal rants Security Varnish
more tags
  • home
  • drupal
  • drupal core commits
  • about

Copyright © 2010 by Heine Deelstra. All rights reserved.