Deny use of the PHP evaluator filter to Anonymous or Authenticated roles. [#1208988]

Despite the obvious security implications, there are many Drupal sites out in the wild that have the PHP evaluator filter enabled for Anonymous users.

It has been proposed that php.module be removed from core. See #1203886: Remove the PHP module from Drupal core

A less drastic alternative would be to protect novice administrators from shooting themselves in the foot by denying use of the PHP evaluator filter to the Anonymous or Authenticated roles.

Comment	File	Size	Author
#4	php_code-1208988-4.patch	2 KB	pillarsdotnet
#4
#3	php_code-1208988-3.patch	2.67 KB	pillarsdotnet
#3
#1	php_code-1208988-1.patch	2.49 KB	pillarsdotnet
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

pillarsdotnet CreditAttribution: pillarsdotnet commented 4 July 2011 at 20:14

Status:

Active

» Needs review

File	Size
php_code-1208988-1.patch	2.49 KB

This patch implements hook_form_FORM_ID_alter() to remove dangerous checkboxes from the user_admin_permissions form.

Comment #2

pillarsdotnet CreditAttribution: pillarsdotnet commented 4 July 2011 at 20:24

Issue tags:

+Security

File	Size
php_code-1208988-3.patch	2.67 KB

Comment #4

pillarsdotnet CreditAttribution: pillarsdotnet commented 4 July 2011 at 21:20

File	Size
php_code-1208988-4.patch	2 KB

Removed the stray chunk from _php_filter_tips().

Comment #5

pillarsdotnet CreditAttribution: pillarsdotnet commented 5 July 2011 at 00:13

Issue tags:

+Needs backport to D7

Comment #6

catch

he/him

English

CreditAttribution: catch commented 5 July 2011 at 02:58

I think this is somewhat a duplicate of #594412: Correctly label all site-owning super-admin permissions. There are similar risks to giving anonymous users full HTML or administer users permissions too.

Comment #7

David_Rothstein CreditAttribution: David_Rothstein commented 9 July 2011 at 20:24

The idea of disabling these checkboxes on the permission page was previously proposed at #248598: Label permissions which are warned about in the user interface and rejected, so it's worth reading up on the reasoning there - I don't think that has changed.

What that issue eventually did instead was label them in hook_permission() with 'restrict access' => TRUE, so that at least a contributed module could use that information to disable or remove the checkboxes or whatever else.

Filter permissions are trickier, though, since you can't usually tell whether or not a text format is configured dangerously, so we never label them with 'restrict access' => TRUE currently. The issue to deal with that properly is #275811: Warn about potentially insecure filter configurations, but that's also complicated. I wonder if the php_list_permissions() function introduced in the above patches could be a good first step towards at least labeling text formats with the PHP filter as dangerous? Among text formats, there are many ways they can be configured to be a security risk, but the PHP filter kind of does stand in a class of its own.