Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#42126] - [5.1] Strengthen Security

Feature ? Language Change NPM Resource Changed PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
12 Oct 2023

Pull Request for Issue # .

Summary of Changes

  • Disable PHP execution inside the media folder
  • Option to apply an .htaccess to each filesystem-local folder (defaults to false for b/c)

Testing Instructions

  • Apply the patch.
  • create a file media/test.php with contents <?= phpinfo();
  • try to access the file, ie localhost/media/test.php (you shouldn't get the phpinfo page)
  • Goto /administrator/index.php?option=com_plugins&view=plugins and edit the filesystem local plugin by enabling the Strengthen Security switch
  • Create a file images/test.php with contents <?= phpinfo();
  • try to access the file, ie localhost/images/test.php (you shouldn't get the phpinfo page)
  • Goto /administrator/index.php?option=com_plugins&view=plugins and edit the filesystem local plugin by disabling the Strengthen Security switch
  • check that the images folder doesn't contain an .htaccess file

Actual result BEFORE applying this Pull Request

PHP execution inside the static folders

Expected result AFTER applying this Pull Request

PHP execution is prevented inside the static folders

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Votes

# of Users Experiencing Issue
0/1
Average Importance Score
5.00
avatar dgrammatiko dgrammatiko - open - 12 Oct 2023
avatar dgrammatiko dgrammatiko - change - 12 Oct 2023
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2023
Category ⇒ Administration com_content com_menus Language & Strings Repository NPM Change JavaScript Front End Plugins
avatar dgrammatiko dgrammatiko - change - 12 Oct 2023
Labels Added: Language Change NPM Resource Changed PR-5.1-dev
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2023
Category Administration com_content com_menus Language & Strings Repository NPM Change JavaScript Front End Plugins ⇒ Administration com_content Language & Strings Repository NPM Change JavaScript Front End Plugins
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2023
Category Administration com_content Language & Strings Repository NPM Change JavaScript Front End Plugins ⇒ Administration Language & Strings Repository NPM Change JavaScript Front End Plugins
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2023
Category Administration Language & Strings Repository NPM Change JavaScript Front End Plugins ⇒ Administration Language & Strings Repository NPM Change Front End Plugins
avatar dgrammatiko dgrammatiko - change - 12 Oct 2023
Status Pending ⇒ Closed
Closed_Date 0000-00-00 00:00:00 ⇒ 2023-10-12 12:22:17
Closed_By ⇒ dgrammatiko
avatar dgrammatiko dgrammatiko - change - 12 Oct 2023
Status Closed ⇒ Pending
avatar dgrammatiko dgrammatiko - close - 12 Oct 2023
avatar dgrammatiko dgrammatiko - close - 12 Oct 2023
a125790 12 Oct 2023 avatar dgrammatiko sec
avatar dgrammatiko dgrammatiko - reopen - 12 Oct 2023
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2023
Category Administration Language & Strings Repository NPM Change Front End Plugins ⇒ Administration Language & Strings Repository NPM Change Front End Plugins Unit Tests
avatar dgrammatiko dgrammatiko - change - 12 Oct 2023
Labels Added: ?
avatar brianteeman
brianteeman - comment - 12 Oct 2023

I am not a fan of this approach although I do agree with its intentions.

Instead of allowing php to be executable in every folder and then adding a deny file in every media folder I would simply suggest adding a deny all in the root htaccess and then people can add exceptions to that rule.

For the core there are only a few exceptions

avatar dgrammatiko
dgrammatiko - comment - 12 Oct 2023

@brianteeman I totally agree that it's better to have secure by default ootb. The only problems are:

  • that it cannot happen in a patch version (if I'm not wrong)
  • it will impact heavily some extensions (and by extension end users)

thus this middle ground PR. Without enabling the new option only the media folder gets the php exclusion, which should have zero effects, as that folder should never have php files.
But if the maintainers feel that going full blast is ok for a patch version then someone with better apache knowledge should take over

avatar brianteeman
brianteeman - comment - 12 Oct 2023

@brianteeman I totally agree that it's better to have secure by default ootb. The only problems are:

  • that it cannot happen in a patch version (if I'm not wrong)

See comment below

  • it will impact heavily some extensions (and by extension end users)

If there really are extensions that allow direct access to any php files anywhere then they are cr*p and I dont care about them. There are very very very few legitimate cases for direct entry points such as the one in core for joomlaupdate

thus this middle ground PR. Without enabling the new option only the media folder gets the php exclusion, which should have zero effects, as that folder should never have php files. But if the maintainers feel that going full blast is ok for a patch version then someone with better apache knowledge should take over

If there are extensions abusing the non-media folders then there is no reason to assume that they are not also abusing the media folder.

I really dont see this as being anything more complex than previous changes to the core htaccess file which have always been for new installs only plus a post-installation message for existing sites. Exactly the same as seen in this PR #30221

avatar dgrammatiko dgrammatiko - change - 30 Nov 2023
Status Pending ⇒ Closed
Closed_Date 2023-10-12 12:22:17 ⇒ 2023-11-30 09:51:53
Labels Added: Feature ?
Removed: ?
avatar dgrammatiko dgrammatiko - close - 30 Nov 2023

Add a Comment

Login with GitHub to post a comment