Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
The php module has been removed from core in Drupal 8 (yay!). However some security concerns. In the issue to #1203886: Remove the PHP module from Drupal core comment#143 @David_Rothstein suggests:
The only "safe" way for a module to execute user-provided PHP code (in such a way that even user 1 cannot directly bypass it) is to:
Check
module_exists('php') && user_access('use PHP for settings')before allowing the code to be entered.
Checkmodule_exists('php')before allowing the code to be executed.
Proposed resolution
...and proposes:
This is best practice currently, but I think we should aim to enforce it (at the security team level) in Drupal 8. The path to that is:
- In Drupal 7, module maintainers have intentionally (and justifiably, I think) resisted the module_exists('php') part of the above, because they don't want to force their users to endure the unwanted side effects (the extra text format, etc) of turning that module on (see #870938: Add new permission for controlling imports for the ctools issue around this). In Drupal 8, last time I checked it's even worse because some Views code had moved into the module too; the module has gotten bigger when it should be getting smaller. To force module maintainers who want to execute PHP to rely on the PHP module, we have to strip it down and split all the side effects and features out of the main module.
- Although the above two code checks aren't that complicated, they are just subtle enough that I think people will manage to get them wrong by accident too. So there could be some benefit to having easy-to-remember API functions as wrappers around the above, something like php_can_be_entered() and php_can_be_executed(). By definition, these API functions would have to live in core. However, I don't think that necessarily means anything else would need to live in core; although it would be unusual for code in Drupal core to check for the existence of a module and permission that live in contrib, it's not completely unprecedented for core code to assume something about contrib modules (at one point I believe there were a few things in core that were specific to the Devel module). I think this is one of the rare cases where it would make sense. (And for extra fun, we could even consider making core use these functions in #932110: On some servers, the Update Manager allows administrators to directly execute arbitrary code even without the PHP module, but that's its whole own discussion.)
Related Issues
Follow-up from #1203886: Remove the PHP module from Drupal core.
Comments
Comment #0.0
ZenDoodles CreditAttribution: ZenDoodles commentedTook out duplicate text accidentally pasted into issue summary.
Comment #1
jhedstromThe PHP module in contrib hasn't had a commit in over a year. Postponing this issue.
Comment #2
gregglesI don't think that the state of php in contrib affects this issue. This issue is about making it easy for developers to figure out if they should run accept/eval some php code entered by an admin. That's true regardless of the php module in contrib.
Comment #3
jhedstromre: #2 ah, that makes sense. Tagging for issue summary update.