Make WordPress Core

Opened 13 years ago

Last modified 8 months ago

#18546 accepted enhancement

Add index.php to wp-includes and wp-admin/includes

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2
Component: Bootstrap/Load Keywords: dev-feedback has-patch
Focuses: Cc:

Description

In ticket:17601:15, dd32 suggested an idea of adding index.php to wp-includes:

/wp-includes/index.php doesnt exist however, leading to that entire folder being indexed by google in some cases (which does happen), this will cause Search Engines to index the contents of these files, leading to the errors being logged.

The patch adds index.php to wp-includes and wp-admin/includes.

Attachments (5)

18546.patch (538 bytes) - added by SergeyBiryukov 13 years ago.
18546.diff (79.1 KB) - added by whyisjake 4 years ago.
18546.2.diff (3.3 KB) - added by whyisjake 4 years ago.
18546.3.diff (86.8 KB) - added by whyisjake 4 years ago.
Gruntfile.js changes and all of the added files. (173 new ones...)
18546.4.diff (3.3 KB) - added by xkon 4 years ago.

Download all attachments as: .zip

Change History (47)

#1 @SergeyBiryukov
13 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Closing in favor of #18465.

#2 @SergeyBiryukov
13 years ago

  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Still might be a good idea, due to #18841.

#3 @nacin
13 years ago

  • Version changed from 3.3 to 3.2

#4 @kurtpayne
13 years ago

  • Cc kpayne@… added

Should more directories be included?

There are 81 other folders + uploads/YYYY/MM folders that are indexed, too. Where do you draw the line?

Full list, for reference:

wp-content/uploads/[YYYY]
wp-content/uploads/[YYYY]/[MM]
wp-content/plugins
wp-content/upgrade
wp-content/uploads
wp-includes/pomo
wp-includes/js
wp-includes/js/thickbox
wp-includes/js/jcrop
wp-includes/js/tinymce
wp-includes/js/tinymce/utils
wp-includes/js/tinymce/plugins
wp-includes/js/tinymce/plugins/wplink
wp-includes/js/tinymce/plugins/wplink/img
wp-includes/js/tinymce/plugins/wplink/js
wp-includes/js/tinymce/plugins/wplink/css
wp-includes/js/tinymce/plugins/directionality
wp-includes/js/tinymce/plugins/wpdialogs
wp-includes/js/tinymce/plugins/wpdialogs/js
wp-includes/js/tinymce/plugins/wpeditimage
wp-includes/js/tinymce/plugins/wpeditimage/img
wp-includes/js/tinymce/plugins/wpeditimage/js
wp-includes/js/tinymce/plugins/wpeditimage/css
wp-includes/js/tinymce/plugins/wordpress
wp-includes/js/tinymce/plugins/wordpress/img
wp-includes/js/tinymce/plugins/wordpress/css
wp-includes/js/tinymce/plugins/spellchecker
wp-includes/js/tinymce/plugins/spellchecker/includes
wp-includes/js/tinymce/plugins/spellchecker/img
wp-includes/js/tinymce/plugins/spellchecker/classes
wp-includes/js/tinymce/plugins/spellchecker/classes/utils
wp-includes/js/tinymce/plugins/spellchecker/css
wp-includes/js/tinymce/plugins/fullscreen
wp-includes/js/tinymce/plugins/media
wp-includes/js/tinymce/plugins/media/js
wp-includes/js/tinymce/plugins/media/css
wp-includes/js/tinymce/plugins/tabfocus
wp-includes/js/tinymce/plugins/paste
wp-includes/js/tinymce/plugins/paste/js
wp-includes/js/tinymce/plugins/wpfullscreen
wp-includes/js/tinymce/plugins/wpfullscreen/css
wp-includes/js/tinymce/plugins/inlinepopups
wp-includes/js/tinymce/plugins/inlinepopups/skins
wp-includes/js/tinymce/plugins/inlinepopups/skins/clearlooks2
wp-includes/js/tinymce/plugins/inlinepopups/skins/clearlooks2/img
wp-includes/js/tinymce/plugins/wpgallery
wp-includes/js/tinymce/plugins/wpgallery/img
wp-includes/js/tinymce/themes
wp-includes/js/tinymce/themes/advanced
wp-includes/js/tinymce/themes/advanced/skins
wp-includes/js/tinymce/themes/advanced/skins/o2k7
wp-includes/js/tinymce/themes/advanced/skins/o2k7/img
wp-includes/js/tinymce/themes/advanced/skins/default
wp-includes/js/tinymce/themes/advanced/skins/default/img
wp-includes/js/tinymce/themes/advanced/skins/wp_theme
wp-includes/js/tinymce/themes/advanced/skins/wp_theme/img
wp-includes/js/tinymce/themes/advanced/skins/highcontrast
wp-includes/js/tinymce/themes/advanced/img
wp-includes/js/tinymce/themes/advanced/js
wp-includes/js/tinymce/langs
wp-includes/js/jquery
wp-includes/js/jquery/ui
wp-includes/js/crop
wp-includes/js/scriptaculous
wp-includes/js/imgareaselect
wp-includes/js/plupload
wp-includes/js/swfupload
wp-includes/js/swfupload/plugins
wp-includes/css
wp-includes/theme-compat
wp-includes/images
wp-includes/images/crystal
wp-includes/images/wlw
wp-includes/images/smilies
wp-includes/Text
wp-includes/Text/Diff
wp-includes/Text/Diff/Engine
wp-includes/Text/Diff/Renderer
wp-admin/maint
wp-admin/js
wp-admin/css
wp-admin/images
wp-admin/images/screenshots

#5 @joostdevalk
13 years ago

My opinion? Do them all.

#6 follow-up: @bpetty
12 years ago

It really doesn't make any difference whether the WP-specific folders not containing user content (possibly sensitive information) are directory indexed or not, but I can certainly see a reason to silence directory indexing on any folders containing installation-specific files (i.e. any folder under wp-content).

So if this is something that is done, I would recommend that only the following folders are included (there might be some other dynamic folders missing here):

wp-content *
wp-content/blogs.dir/{$blog_id}/files (see multisite)
wp-content/cache (see RSSCache class)
wp-content/languages (see update_core())
wp-content/mu-plugins
wp-content/plugins *
wp-content/themes *
wp-content/upgrade (might not be necessary if only used for core update)
wp-content/uploads
wp-content/uploads/[YYYY]
wp-content/uploads/[YYYY]/[MM]

* These are already silenced.

Silencing wp-includes or wp-admin is really completely pointless, and doesn't help secure anything or hide any sensitive content.

#7 follow-up: @ericlewis
12 years ago

I wonder if something can be done at the rewrite module level to nix directory indexing.

#8 in reply to: ↑ 7 @bpetty
12 years ago

  • Cc bpetty added
  • Component changed from General to Security
  • Keywords dev-feedback added
  • Priority changed from normal to low
  • Severity changed from normal to minor
  • Type changed from defect (bug) to enhancement

Replying to ericlewis:

I wonder if something can be done at the rewrite module level to nix directory indexing.

I don't think this can be tackled with mod_rewrite rules. Folders themselves can be requested from the server without a trailing slash, so it would require conditionals to test if the requested resource is a folder, and that could open up major performance problems for regular, valid requests across the wp-content folder.

An alternative solution along this kind of thinking though would be using "Options -Indexes" in .htaccess, however, many Apache server configurations don't allow this in .htaccess. Using index.php is much more effective. On the other hand, Apache won't complain if it sees this option even though it can't be disabled (under AllowOverride None configurations with +Indexes), so that couldn't hurt, but there also might be some plugin that uses a folder under wp-content that is intended to be indexed, so that might not be ideal either.

#9 in reply to: ↑ 6 ; follow-up: @SergeyBiryukov
12 years ago

  • Component changed from Security to General

Replying to bpetty:

Silencing wp-includes or wp-admin is really completely pointless, and doesn't help secure anything or hide any sensitive content.

This isn't for security, the idea was to prevent unnecessary crawling and irrelevant errors in server logs.

For the most part, this is fixed in #18465, but not for installations in subdirectories or without .htaccess, in which case do_robots() doesn't get called.

Another way to address this might be trying to create .htaccess file on install (which was previously suggested for #6481) if it doesn't exist yet and the permissions allow to do that. This would have an additional bonus of displaying 404 pages in theme layout even with default permalinks.

Last edited 12 years ago by SergeyBiryukov (previous) (diff)

#10 in reply to: ↑ 9 @bpetty
12 years ago

Replying to SergeyBiryukov:

Replying to bpetty:

Silencing wp-includes or wp-admin is really completely pointless, and doesn't help secure anything or hide any sensitive content.

This isn't for security, the idea was to prevent unnecessary crawling and irrelevant errors in server logs.

In my experience, the only reason to explicitly override server configuration settings like this is for increased security measures. WordPress (and any web application software in general) should not be doing anything in regards to overriding server configuration behavior otherwise, and I guess that's not what others here think. Allow me to explain why I re-classified this as security then.

If a PHP script is returning errors because it was requested directly, there's always another 100% effective way of handling those errors regardless of what httpd is in use. Web applications like phpBB deal with this using a simple check in every include, which is very helpful for security in general too:

if (!defined('IN_PHPBB')) { exit; }

I've known this approach to be pretty standard across numerous web applications when it is impossible to leave include files outside of the document root.

That said though, I know that it's possible that plugins have directly included WP core include files without going through wp-load.php (using custom ajax functionality, image thumbnailing, or who knows what else). In that case, just actually handling the error appropriately (depending on what it is) *is* still the ideal solution that works everywhere. If there's an error being generated, who knows what else the script is doing inappropriately because it was requested directly, trying to cut back on crawlers from hitting those files isn't going to fix it. The file should be making the appropriate checks that classes/methods defined elsewhere exist, or that it was included from another file if it's only meant to be used from those files (when it depends on something defined or handled in those files) whenever it does anything beyond just defining new classes, methods, constants, or variables for use by other scripts.

And in regards to non-error-related requests, if someone is really concerned about their server logs and these kinds of requests/crawlers, they should be tuning their httpd configuration directly, especially in regards to automated directory indexing since it is in fact the httpd generating those pages, not WordPress.

Another way to address this might be trying to create .htaccess file on install (which was previously suggested for #6481) if it doesn't exist yet and the permissions allow to do that. This would have an additional bonus of displaying 404 pages in theme layout even with default permalinks.

Not that I like a solution like this because it really doesn't solve the root problem (regardless of what rules are being used), but #6481 doesn't mention indexing at all. What kind of .htaccess configuration options or rewrite rules are you talking about that address this?

Anyway, this is the first time I've read "it was decided that path disclosure was not a classified as a security vulnerability for WordPress" (ticket:17601:15). If that's the case, this entire ticket is invalid, and technically speaking, index.php should be removed from the three locations that it's already been added for.

#11 @nacin
11 years ago

  • Component changed from General to Bootstrap/Load

#12 @chriscct7
9 years ago

  • Keywords needs-refresh added
  • Priority changed from low to normal
  • Severity changed from minor to normal

#14 @swissspidy
8 years ago

Another way to address this might be trying to create .htaccess file on install (which was previously suggested for #6481) if it doesn't exist yet and the permissions allow to do that. This would have an additional bonus of displaying 404 pages in theme layout even with default permalinks.

Since #6481 has landed in 4.2, how relevant is this one?

#15 @SergeyBiryukov
7 years ago

#41790 was marked as a duplicate.

#16 @ocean90
5 years ago

#50077 was marked as a duplicate.

#17 @ocean90
5 years ago

#50076 was marked as a duplicate.

#18 follow-ups: @jonoaldersonwp
5 years ago

Given that there's been some recent conversation about this (#50076), and as this ticket seems to be a bit stale, it's worth revisiting that:

  • We can't reliably rely on server/hosting setups to manage this well.
  • It's a persistent SEO and (arguably, though secondarily) security risk.
  • Adding index files to all folders is the only viable/reliable/universal solution to this.
  • (and in an ideal world, we'd also have those index files trigger the themes' 404 template and return a 404 HTTP status).
  • Yes, it's a challenge to enforce this across customisations/plugins/themes, but, some progress is better than none.

TL;DR, "yes, we need to add index files to all folders".

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#19 in reply to: ↑ 18 ; follow-up: @Otto42
5 years ago

Replying to jonoaldersonwp:

Adding index files to all folders is the only viable/reliable/universal solution to this.

This is true and I agree. It is unfortunate that it is necessary for web applications to need to solve what is essentially a server configuration error, and it feels kinda hacky, but if including some zero byte files fixes the issue more effectively than education, then so be it.

Question: Should it be index.php, or index.html? Or index.htm? What provides the most coverage? Might as well get it right the first time.

#20 in reply to: ↑ 18 @AnotherDave
5 years ago

@jonoaldersonwp - agreed and I appreciate you posting / supporting the need for index files to be present in all the core folders. While I am new to WordPress Trac, I'm definitely not new to server & site security. I've been running a small hosting service for over two decades and have done thousands of WordPress migrations & installs (among other PHP scripts).

A lot of WordPress installs are on shared hosting and the host cannot be expected to implement a global measure - such as setting the servers to block access to all folders that do not contain an index file - since that would create issues for advanced users who are using their hosting for more than just WordPress and have created other important folders in their hosting accounts that they keep private and have a need to NOT have an index file in them.

@Otto42 - Please don't take me as being argumentative when I say this is not a server configuration error - if a shared host were to globally block access to folders that do not contain an index file, not only would it break certain needed functionality for some clients, but it would also generate a ton of support tickets to the host. Instead, the best solution is PHP scripts such as WordPress to already have an index file in all of their core folders.

As far as choosing index.php or index.html or index.htm - it won't make a difference which extension you use on a properly configured server, any one of them will do the job. The only consideration here really is the priority in which the host has their servers set to in regard to which extension takes precedence over the other. For example - my servers are set up so that the order is like this:
index.html
index.htm
index.php
Which means if an index.php file exists in the same folder as an index.html file, when a bot or browser visits that folder, it will load the index.html file instead of the index.php file (unless the index.php file is directly called / accessed). The reason I have that "pecking order" set on my servers is because some of my users are old-school and others just prefer to use a blank index.html in their public_html folder while setting up a new WordPress in scenarios when the site is actually live and getting traffic, to prevent current visitors from setting a WordPress install page. Another good reason to have index.html set as the priority over index.php is when a user has an old HTML website and they're making the transition to WordPress - it allows them a seamless transition, keeping their HTML site live while their new WordPress site is being developed in the background.

In any case, when it comes to protecting a folder such as /wp-includes/css/ (which by default does not have an index file) , it doesn't matter if your blank index is a .html or .htm or .php (however, I would go with .php so that once WordPress does finally address this, their .php won't be overridden by .html) - but what matters is that there's a blank index there to prevent those contents from being indexed by search bots and so easily accessible to anyone. Not having an index file in that folder risks information leakage, in some cases hotlinking for malicious purposes, and will cause a site to rank lower on security test / check sites such as https://sitecheck.sucuri.net , and potential PCI Compliance issues if they're an e-commerce site.

Apologies for the long post, but I hope someone finds the info useful.

WordPress is an amazing piece of software on many levels and it's a fantastic solution for my clients of all types, so I'm not criticizing it. I'm asking - how hard would it really be for it to include a blank index.php file in each folder upon next version release, as opposed to over 8 years of users having to figure out what to do for their own protection? ;-)

#21 @Otto42
5 years ago

@AnotherDave Yes, I agree that this needs to be done.

The reason I asked the question about the extension was because it would be useful to find out a variety of host default configurations in order to use the one that will most effectively impact the most systems. WordPress runs on a wide variety of hosting services, so knowing what the most common configurations are for the index order would let us determine which would be the best extension to use. If some hosts don't do .htm in there, then we shouldn't use that one.

Hosting configurations are often weird, so knowing edge cases always helps. The most likely candidate of simply using index.php is probably fine, since if PHP execution isn't enabled then the site doesn't work anyway. But using index.html, for example, would prevent directory browsing even if PHP was misconfigured on a host.

#22 @joostdevalk
5 years ago

Using index.html has the added benefit of not allowing those files to be targeted by hackers for code execution.

#23 @AnotherDave
5 years ago

Indeed, until WordPress has index.php files in the folders of the core package, index.html is a safe choice.

I can't imagine any host not having .html enabled, but there might be some that don't have .htm enabled. (And if you're using WordPress, they obviously have .php enabled).

Knowing the priority on the server - your host can easily tell you the priority order of index file extensions on your server, just ask them.

#24 in reply to: ↑ 19 @SergeyBiryukov
5 years ago

Replying to Otto42:

Question: Should it be index.php, or index.html? Or index.htm? What provides the most coverage?

We already have three index.php files in wp-content with the // Silence is golden comment:

wp-content/index.php
wp-content/plugins/index.php
wp-content/themes/index.php

Since WordPress itself requires PHP, it seems better to stick to the existing pattern of using index.php for consistency. It would also open a possibility for us to address this point from comment:18 in the future:

  • (and in an ideal world, we'd also have those index files trigger the themes' 404 template and return a 404 HTTP status).

#25 @jonoaldersonwp
5 years ago

Yeah, I'd be keen on .php for the same reasons.
Returning a (blank) 404 is only a partial 'fix' from an SEO and accessibility perspective. If we can pave the way to returning more user-friendly error scenario, that'd be preferable.

#26 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.5
  • Owner set to SergeyBiryukov
  • Status changed from reopened to accepted

#27 @ocean90
5 years ago

If this really needs to be done (I'm still in in doubt since it can't cover everything and is similar to path disclosure) then there should be a grunt task added which does this automatically and only for the build directory.

#28 follow-up: @joostdevalk
5 years ago

Why complicate things like that @ocean90?

What’s wrong with a few added files?

#29 in reply to: ↑ 28 @ocean90
5 years ago

Replying to joostdevalk:

Why complicate things like that @ocean90?

What exactly is complicated for you? Nobody is telling you to implement this.

During the build task we already move files around so you can't cover all directories in /src. Also, over the last years we have added more libraries so the listed directories here aren't up to date. And we'll probably add/update more in the future. It should not be a manual task to check for missing index files. That's what build tasks are for.

@whyisjake
4 years ago

#30 @whyisjake
4 years ago

It looks like it is 243 new files.

Programatically add a file to every folder:

find src -type d -exec touch {}/index.php \;

I took all of the new files and then did this:

echo "<?php\n// Silence is golden.\n?>\n" >> path/to/index.php

And then composer format to ensure that we had the correct formatting for the diff.

I agree that it would be great to ensure that we had a build process to ensure that we had something like this when new packages/folders are added.

#31 @SergeyBiryukov
4 years ago

  • Milestone changed from 5.5 to 5.6

I don't think the index.php files should be added to external libraries:

  • js/_enqueues/vendor
  • wp-includes/ID3
  • wp-includes/PHPMailer
  • wp-includes/Requests
  • wp-includes/SimplePie
  • wp-includes/sodium_compat
  • wp-includes/Text

Moving to 5.6 to work on a build task to handle this in a consistent way when new directories are added.

@whyisjake
4 years ago

@whyisjake
4 years ago

Gruntfile.js changes and all of the added files. (173 new ones...)

#32 @ayeshrajans
4 years ago

I would very much love to see an htaccess file solution as suggested earlier. Options -index works quite well in Drupal, for example.

#33 @hellofromTonya
4 years ago

  • Keywords needs-refresh removed

Latest patch applies cleanly.

#34 @peterwilsoncc
4 years ago

In 18546.2.diff can the folders task create the files in the build directory only?

My very strong preference is to avoid committing any auto-generated files, it's problematic with the few WordPress has already, so adding more problems isn't ideal.

#35 @SergeyBiryukov
4 years ago

  • Milestone changed from 5.6 to 5.7

Didn't get a chance to review this in time for 5.6 Beta 1, moving to the next release for now.

This ticket was mentioned in Slack in #core by lukecarbis. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#38 @hellofromTonya
4 years ago

  • Milestone changed from 5.7 to Future Release

With 5.7 Beta 1 landing in less than 2 weeks, punting this ticket to Future Release.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

@xkon
4 years ago

#39 @xkon
4 years ago

This looked really interesting after todays devchat (I know it went to Future but I wanted to check it out).

18546.4.diff makes some minor adjustments on top of 18546.2.diff and also adds the build:protect_folders (sorry for renaming :D) as the last step of npm run build as mentioned in comment 34.

Since we're checking for existing index.php and also have the wx flag I changed the folder arrays also to exclude only the libraries mentioned in comment 31. The additional folders from the previous patch already contain an index so they should be fine from the checks :) ( fingers crossed haha ).

---

Edit: Thinking about it, since we're going for the /build directory, do we really have to exclude any folders? I think it's just better to add an index everywhere if it doesn't exist.

Last edited 4 years ago by xkon (previous) (diff)

#40 @SergeyBiryukov
9 months ago

#60848 was marked as a duplicate.

#41 @Website Rob
9 months ago

Since my ticket (#60848) was marked as a duplicate and has revitalized this ticket, I will add my comments here.

The Internet was built on serving .htm files and almost every server OS in the world will do that. WordPress should be concerned with best practices of how WordPress itself works within a website, not how a particular server is setup. In my world, having an index.htm file in every default WordPress directory would be considered as best practice. There are many good reasons to have an index.htm file in every directory and it does no harm.

In this regard and in a perfect world, WordPress core "could" concern itself with whether or not every Theme, Addon, whatever, has their own index.htm file, but I think not. As we do not yet live in a perfect world, best to focus only on WordPress default directories. As servers are setup by default to serve an index file first; regardless of the format, having more than one index file (index.htm, index.php) in every directory is not a bad thing and does no harm--and it can be blank.

Please correct me if I'm wrong, but at this point in time and into the far future, I cannot see any downside for adding an index.htm file into every default WordPress directory.

#42 @SergeyBiryukov
8 months ago

#61003 was marked as a duplicate.

Note: See TracTickets for help on using tickets.