colorbox-form security issue [#778102]

The issue below was reported to the Drupal security team. Their recommendation was to create an issue in the public queue since there is no stable release of colorbox yet.

ISSUE:
colorbox module offers an option to

enable custom links that can open any form in a Colorbox. Add the class "colorbox-form" to the link and build the url like this /colorbox/form/[form_id]?width=500&height=500"

The callback for this path colorbox/form (line 66) only checks for user_access('access content'). If content is mal-created with a colorbox/form link to an admin form, then an anonymous user with 'access content' can click on link and change admin settings.

EXPOSURE:
all modules that have admin form functions in their .module file and
not in an .admin.inc file

TEST:
[1] grant 'access content' permission to anonymous user
[2] install extlink module
[3] create content with the link below:

<a class="colorbox-form" href="/colorbox/form/extlink_admin_settings?width=500&height=500">Test hack</a>

[4] visit site as anonymous use

OTHER LEAKS TO PUBLIC:
#760972: Allow a selection of forms to be opened in a Colorbox via a custom formatted link - this discussion almost exposes this security hole to the public

VERSIONS:
Drupal: all versions of 6

colorbox: 6.x-1.x-dev, 2010-04-19, datestamp = "1271635394"

/************************************************************************************/
Drupal Security Team response:

That's a serious issue.

However, the colorbox module only has a dev and a beta release. Our policy is to treat all issues that don't concern stable releases in the public issue queue.

Please create an issue about this at
http://drupal.org/node/add/project-issue/colorbox

Regarsd,
Drupal security team

/************************************************************************************/

My thoughts

To simplify, limit this functionality to only a few chosen forms with the proper access checks - ie login, contact, ..

Comments

Comment #1

frjo CreditAttribution: frjo commented 22 April 2010 at 05:58

Assigned:

Unassigned

» frjo

Thoughtless of me to add this feature and good of you to catch it.

A whitelist of forms with access checks as you suggest sounds like the best solution.

Here are three I suggest, any more?

contact_mail_page
user_login
user_login_block

Comment #2

frjo CreditAttribution: frjo commented 22 April 2010 at 06:04

More forms:

user_register
user_pass

Comment #3

frjo CreditAttribution: frjo commented 22 April 2010 at 12:55

Status:

Active

» Needs review

Committed a fix to 6-dev that I hope will remove this security problem.

The main part is this access check that will only allow certain forms and check if the user should have access to them.

/**
 * Colorbox menu access check.
 */
function _colorbox_form_page_access($form_id) {
  switch ($form_id) {
    case 'contact_mail_page':
      $access = user_access('access site-wide contact form');
      break;
    case 'user_register':
      $access = user_register_access();
      break;
    case 'user_pass':
    case 'user_login':
    case 'user_login_block':
      $access = user_is_anonymous();
      break;
    default:
      // All other forms get no access.
      $access = FALSE;
  }

  return $access;
}

Please try it out and report back here.

~~The tarballs for dev is only rebuild every 12 hour so grab the code directly from CVS.~~

The tarball has been rebuilt now so just download it from the project page as normal.

Comment #4

recrit CreditAttribution: recrit commented 22 April 2010 at 14:40

One small tweak to the contact page is below in the results. Other than that its a great step forward. Its a useful functionality that can be safely extended now as other forms are suggested. The forms in the white list should be added to the admin description so they don't blindly try to load any form and fill up the issue queue when it doesn't work.

Results:

test hack with extlink_admin_settings: PASSED - results in a colorbox with text "Request unsuccessful."
user_login: PASSED - anonymous user results in login form colorbox, authenticated user receives "Request unsuccessful."
contact_mail_page: ??? - something odd here, it appeared to try load everything on my contact page, blocks and all, then redirected me to a white screen... this might just be something with my set up. I have similar code running fine for a contact form block, so not sure whats going on.
Either way this needs revamped a little bit. This should call the contact wrapper function 'contact_site_page' which includes a flood control check before getting the form 'contact_mail_page'.
Something like this would work:
```
function colorbox_form_page($form_id) {
  $GLOBALS['devel_shutdown'] = FALSE; // Prevent devel module from spewing.

  // Load includes for certain common forms.
  switch ($form_id) {
    case 'contact_site_page':
    case 'contact_mail_page':
      module_load_include('inc', 'contact', 'contact.pages');
      print contact_site_page();
      break;

    default:
      $form = drupal_get_form($form_id);
      if (!empty($form)) {
        print $form;
      }
  }

  exit;
}
```