DevinVinson / WordPress-Plugin-Boilerplate

[WordPress] A foundation for WordPress Plugin Development that aims to provide a clear and consistent guide for building your plugins.
http://wppb.io
7.67k stars 2.25k forks source link

Consider Removing index.php Files #319

Closed robneu closed 9 years ago

robneu commented 9 years ago

I think this is a bad practice and can actually cause more harm than good. For one thing, security through obscurity is generally considered to be a bad practice, but aside from that directory listing should be enabled or disabled on servers. It's not something that should be hacked around by plugins or other bits of code. WordPress core doesn't do this, so I don't see any reason for plugins to attempt it.

The only thing these files do is give someone a false sense of security and clutter the plugin with unnecessary files. If 40% of all plugins on a given site used this approach it would leave 60% of plugin directories indexed. IMO it would also decrease the likelihood of the issue actually being fixed since 40% of the plugin directories appear to be fine.

If the maintainers disagree with me, that's fine. I just figured I'd bring it up as something worth considering.

robneu commented 9 years ago

One other note is that even if you decide to keep these, the one in the root directory should be removed. See this comment for further explanation.

jjeaton commented 9 years ago

:+1: to this from me. Agreed that this should be controlled on the server level. I dislike littering the plugin with all these empty index.php files

DevinVinson commented 9 years ago

Good points. I thought it had been established best practice and never really challenged that ideal.

I'll go see if I can find some reference within core or the handbook to add additional justification but I'm definitely in favor of this based on your initial points and reference above :smile:

pix2D commented 9 years ago

WordPress core actually does do this, within wp-content at least. And I don't think 1 (or 10 for that matter) empty index.php file in the plugin root takes much processing time away either.

There's plenty of terrible hosting companies out there that don't care and leave it enabled by default and most users simply don't know / care about it at all.

Why make it easier for automated scripts to scan plugin directories for files with known vulnerabilities?

robneu commented 9 years ago

I stand corrected about core but I still don't see any value in this. It doesn't look like core does it anywhere other than wp-content and a few sub-directories.

If people don't care about security and they're using terrible hosting, preventing the listing of a directory isn't going to do much to stop attackers. I guess my main argument is that all you're doing is adding unnecessary cruft to your plugin and reducing the chances that users discover and fix underlying security problems by masking them.

In any event, an alternative fix for the file in the plugin root could be use an .html extension to prevent WP from scanning it for plugin headers.

pix2D commented 9 years ago

It's not that people don't care, it's that they simply don't know any better. We're developers so to us it's obvious. To normal end users that just use WP as a platform for their website / blog it is not.

WP core only does it in wp-content because that's where non core files reside and that's where it would make sense to do it.

In the grand scheme of things it's a non-issue in my opinion. If we can prevent one bot from taking down a users site by not showing directory contents and thus exposing some out of date library used in the plugin then in my mind it is worth it.

It's not about security through obscurity, it's about doing as much as possible to protect end users from things they don't even know they need protecting from.

And how much processing power is really used here by scanning empty index.php files? I'm sure if you were to do some testing you'd be hard pressed to find any difference at all.

robneu commented 9 years ago

Most attackers are going to ping sites with an attack, not a check to see whether or not their attack would work. Why would you waste time and risk alerting system admins by checking to see if an attack will work? You can just run the attack and it will either pass if the vulnerability is there or fail if it isn't.

The original ticket for adding the files in core wasn't for security but for preventing Google from indexing the plugin directories. If that's the goal of doing this, then I guess that's fine and should be left in, although I still feel this is a waste and should be done on the server, but it's not going to stop attackers.

I'm sure it's not using a ton of processing power, but this is meant to be a default boilerplate. Whatever the amount is could be multiplied many times over on a given site because lots the plugins installed were created using this as a starting point.

Regardless, I think the maintainer should at least change the root index file to use an .html extension to save whatever processing power is being wasted there. It should do the same thing as the index.php file so I'm not sure what the argument against doing that would be.

AlchemyUnited commented 9 years ago

@pix2D said "In the grand scheme of things it's a non-issue in my opinion."

you might want to check it out, if you haven't seen it already :)

http://blog.capwatkins.com/the-sliding-scale-of-giving-a-fuck

DevinVinson commented 9 years ago

After doing some further reading I'm kind of split on this. My first instinct was and is to remove them as I noted above since I agree with your original main points.

On the other hand, this is a reflection of core values and best practices. The slight assistance in security, even if only perceived is probably worth keeping by default.

Ultimately the boilerplate is a starting point and not set in stone. Anyone can remove and edit things as they see fit. For now I'll leave them even though I do agree with @robneu that they are not strictly needed.