WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 7 years ago

Last modified 7 years ago

#1038 closed defect (bug) (wontfix)

Limit access to php files

Reported by: anonymousbugger Owned by: matt
Milestone: Priority: normal
Severity: normal Version:
Component: Security Keywords: needs-patch
Focuses: Cc:

Description

Please provide htaccess files or a sample apache configuration for limiting access to php files. E.g. nobody ever should need to open wp-includes/*php directly.

Access can be limited with the <Files> and <FilesMatch> directives.

Change History (22)

comment:1 @anonymousbugger10 years ago

  • Patch set to No

comment:2 @matt10 years ago

  • Milestone set to 1.5.2
  • Owner changed from anonymous to matt
  • Status changed from new to assigned

We can do a if ( !defined() ) exit; at the top of all the wp-include files.

comment:3 @skippy10 years ago

  • Keywords bg|needs-patch added

comment:4 @gregh10 years ago

The .htaccess limitation is a good idea in addition to the PHP-level protection. If the PHP configuration on the server ever gets broken (can easily happen during system upgrades) then the PHP files could end up getting servered to the browser as plain text in which case the "if (!defined())" check will not even execute. Could be a security hole if any sensitive information is stored in the PHP source (database passwords, sensitive file system paths, for example).

comment:5 @gregh10 years ago

At the very least wp-config.php should be protected:

<Files "wp-config.php">
  Order Deny,Allow
  Deny from all
</Files>

You can explicitly whitelist as follows:

<Files "index.php">
  Order Allow,Deny
  Allow from all
</Files>

You can also use wildcards, regexp etc. More docs here:

http://httpd.apache.org/docs/mod/core.html

comment:6 @markjaquith10 years ago

  • Version changed from 1.5 to 1.5.2

I like the idea of PHP protection, because that'll work regardless of the server setup.

comment:7 @matt10 years ago

  • Milestone changed from 1.6 to 2.1
  • Priority changed from normal to low

comment:8 @abhay10 years ago

htaccess is fine and dandy for those using apache servers. i think the f ( !defined() ) exit; is much more versatile.

comment:9 @szepter10 years ago

I think there's no need for a patch, since a properly configured HTTP server doesn't provide direct access to the PHP source code.

For example if you access the wp-config.php file there is no security violating content put out by the server, all you can see is an empty page. Improperly configured HTTP servers therefore should be reconfigured properly, instead limiting the access to these files because it's a "security hole" in the server, not in WordPress itself.

But maybe these sensitive files could be protected by general mod_rewrite rules ? I'm thinking of something like the trackback spam solution: http://blog.mytechaid.com/archives/2005/03/09/wordpress-trackback-spam-solution/.

If you think of something like if ( !defined(...) ) you could simply leave everything as it is, because there is, as I said above, no content put out by accessing wp-config.php.

comment:10 @war5931210 years ago

I think a plug-in would be better. ;)

Like stop people from accessing files like wp-config and show 403 page and perhaps even ban the user. Perhaps make that an option in the admin-cp interface...

And of course a nice log in admin-cp of all the dumb nuts trying to access private files.

cya,

Will

comment:11 @robmiller9 years ago

I think there's no need for a patch, since a properly configured HTTP server doesn't provide direct access to the PHP source code.

It'd be nice to block access to wp-config.php "just in case"; if something goes wrong with Apache and it stops parsing PHP files, someone could fetch your config without you knowing. Sure, it's an unlikely thing but considering the tiny amount of effort required to disallow access it seems worth it.

comment:12 @davidhouse9 years ago

  • Priority changed from low to lowest
  • Severity changed from enhancement to trivial

comment:13 @szepter9 years ago

It'd be nice to block access to wp-config.php "just in case"; if something goes wrong with Apache and it stops parsing PHP files

It's improbable that something goes wrong with Apache and it stops parsing PHP files ... but if so, defines in the wp-config.php or wherever wouldn't solve that problem. These files will be available either way.

In case of the PHP config stops working - I think - also .htaccess-limitations wouldn't join.

The same applies to the plugin solution. This plugin wouldn't be executed if something goes wrong with Apache.

comment:14 follow-up: @thunderlove9 years ago

  • Version changed from 1.5.2 to 2.0.2

With the exception of wp-config, the problem is not so much a hacker viewing the (open-source!) source, but rather some hacker executing the source. Almost certainly, there are some hitherto unknown (or yet-to-exist, or extant [e.g. php registered globals]) bugs/exploits the include files, that only manifest themselves when the file is accessed directly, and only under certain circumstances, or certain server configurations, and could result in DOS attack, spam, or worse.

<?php defined('__WP__') || die('');

It's easy -- easy enough for this strategy to be readily adopted by module and plugin writers -- and it makes things a bit safer, now and in the future.

Lets be clear on smart security coding practices -- if all environment safety checks occur in wp-settings.php, then all executed php should be required to first go through wp-settings.php. This way, all past, present, and future security checks (such as the $_REQUESTGLOBALS? check) are ensured to be in place. And if a file is not designed to be accessed directly, it should not be allowable to access it directly. 'What is not explicitly permitted is denied'

So wp-config isn't the problem -- we cannot actively prevent it from being served raw, and when cooked it includes wp-settings.php. The definition of __WP__ could even go into wp-config.php itself.

"It's improbable that something goes wrong with Apache and it stops parsing PHP files"

Unfortunately, it does happen on occasion. Surely I'm not the only one here who's ever visited a website, and have the php source pop up. Bad sysadmin or user error, certainly. And yes, there is little that can be done about that here.

And while I certainly agree -- we cannot prevent wp-config et al. from being served raw, we certainly can provide additional up-front information and configuration options to users to help prevent it.

For example, regarding the <Files> directives, I would recommend adding them (well, at least the one protecting wp-config.php) to a security.txt, or a install.txt, or a link in readme.html. True, they only apply to apache setups. I do not know anything about other servers, but I should think they too would have some strategy for excluding files and directories, which should also be included.

And there are other possible solutions, too, depending on the server configuration. For example, a set of constants could be defined in wp-config, such as INCPATH and ADMINPATH, to define the absolute path to the given include directories. (wp-settings.php defines WPINC as a relative path, but it seems that strategy was not applied universally). On many hosts, the include files could then be easily moved outside of webspace altogether, along with the database passwords from wp-config. And that would be the best solution to the posted problem.

comment:15 @szepter9 years ago

On many hosts, the include files could then be easily moved outside of webspace altogether, along with the database passwords from wp-config.

Most hosting providers don't allow accessing files or writing to files outside of the docroot, which is the "webspace" in most cases.

Altogether I would say these nice security advice to WP users would make things more complicated ("You should place files there or there and modify the paths there and there ... otherwise things can go wrong ..." and so on ...). Well, these advice are really nice, but not for users who are new to WP.

If Apache stops parsing PHP files under some improbable circumstance, these files wouldn't be executed, the code is served raw, nothing can happen, except that you can read the source code, but that's something you can do much easier by downloading the latest WP version from the website.

I think it's no use overloading code for providing a non-trivial protection system that only works under Apache or under specific circumstances.

comment:16 in reply to: ↑ 14 @foolswisdom9 years ago

  • Keywords needs-patch added; bg|needs-patch removed
  • Milestone 2.1 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

Closing ticket as WONTFIX. Ticket has not been updated in 7 months.

It seems two issues are discussed in this bug:

  1. What if I web server problem resulted in served up php files being displayed in plain text, specifically wp-config.php
  2. Source php files in wp-include being directly called for malicious purposes

The discussion has not resulted in any patches, though it sounds like <?php defined('__WP__') || die(''); would be accepted for the appropriate php files.

Changing to enhancement b/c no real issue shown to be defective. As the priority of "lowest" reflects, and the lack of recent "progress", does not seem worth keeping this ticket open longer.

comment:17 @thenlich7 years ago

  • Priority changed from lowest to normal
  • Resolution wontfix deleted
  • Severity changed from trivial to normal
  • Status changed from closed to reopened
  • Version changed from 2.0.2 to 2.5

This bug is related to #1335 and exposes the server to path info disclosure for most servers, since (unfortunately) the default PHP setting for display_errors is 1.

Path info disclosure is not a trivial issue, as it provides an attacker with vital information for exploiting potential security holes.

In addition, path info disclosure is only one symptom, other issues might exist (for example in plugins) if a PHP file which should only be included is called directly.

Suggested fix:
The only secure method is to put all include files in a separate directory structure, definable in wp-config.php (e.g. WPINC) so that security-conscious admins can install this part in a directory outside the docroot of the webserver or disallowing access with other means (.htaccess) for this stuff!

It is not difficult to do this change, it only requires the include path to be configurable in a central location.

Current situation is that is a real defect in the application, and it is worth fixing this, so I reopen.

comment:18 @Viper007Bond7 years ago

IMO, if you're that worried about paths, then you should have error reporting off. And the path is only a "problem" if you're on a shared server with poor security.

Recommend "wontfix".

comment:19 @thenlich7 years ago

Setting display_errors = 0 is a workaround, which is not always possible, as it requires write access to php.ini. And shared servers do exist, so it is a real problem.

Admittedly, the path info disclosure is not an exploitable security hole by itself (only in combination with other defects), so instead of "wontfix" I recommend changing this into an enhancement rather than a defect.

Do not recommend to close the ticket simply because a workaround exists.

comment:20 @Viper007Bond7 years ago

  • Resolution set to wontfix
  • Status changed from reopened to closed

I can personally live with it being an enhancement rather than defect -- pet peeve of mine where WordPress is blamed for improper sever configuration.

Anyway, your suggestion is different though than the original issue/suggestion, so probably best to open a new ticket rather than hijacking this old ass one that's about a different issue. ;)

comment:21 @Otto427 years ago

  • Version 2.5 deleted

It still might make sense for WordPress to add a statement to the .htaccess like this:

<Location /wp-includes>

Order Deny,Allow
Deny from all

</Location>

Sort of as a preventative measure. Is there any particular downside to doing this that I am unaware of?

comment:22 @DD327 years ago

Sort of as a preventative measure. Is there any particular downside to doing this that I am unaware of?

If i understand that correctly, its to block all HTTP access to the folder?

There are however Javascript and Images in the wp-includes folder which are used in various places.

Note: See TracTickets for help on using tickets.