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.
// 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();
}
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();
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();
}
Hi, I just did a search in a
peach - all drupal themes (not verified) — Fri, 21/03/2008 - 10:26Hi, 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?
maybe
greggles (not verified) — Fri, 21/03/2008 - 14:09It depends what comes on the following lines.
if it just does
drupal_access_denied();
something_else_important();
return;
}
other_stuff();
Then the other_stuff is protected by the return on the following line.
I want to know how Heine has
xamox (not verified) — Sun, 23/03/2008 - 03:06I 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.
excellent
Pedro Pablo (not verified) — Thu, 17/07/2008 - 00:00it 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
In a lot of cases you won't
Xano — Sat, 20/09/2008 - 00:27In a lot of cases you won't be able to end the request by returning
drupal_access_denied()ordrupal_not_found(), because the function these calls are made in are no direct menu callback. The solution is to simplyexit;right after calling one of the aforementioned functions.Post new comment