I just found a really ugly security bug:
The theme configuration pages (e.g. admin/build/themes/settings/garland) are not secured at all against anonymous users.

The enclosed patch introduces the necessary access check (user_access('administer site configuration')) into _system_themes_access.

To reproduce: Just log off your site and go to admin/build/themes/settings/garland. The page will be displayed, as if you had admin rights.

The enclosed patch was created against D6, but does also cleanly apply to HEAD (D7). D5 is not affected by this bug.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

Status: Needs review » Reviewed & tested by the community
jgoldberg’s picture

Has anyone considered some kind of security mechanism that applies to any /admin/* menu item?

cburschka’s picture

Technically, D6 menus should default the access control on items to their parents. admin/build is surely secured. How does this bug occur in spite of that?

cburschka’s picture

Oh, never mind. The access only defaults to the parent if the access callback isn't set - and it is here. If you implement your own access hook, you have to take care to secure the page against unauthorized entry, so the fix in the patch is completely correct.

Gábor Hojtsy’s picture

Version: 6.x-dev » 7.x-dev

Huh. Thanks, committed to 6.x. Also RTBC for 7.x.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.