WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#18546 reopened enhancement

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

Reported by: SergeyBiryukov Owned by:
Milestone: Awaiting Review Priority: low
Severity: minor 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 (1)

18546.patch (538 bytes) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (12)

SergeyBiryukov3 years ago

comment:1 SergeyBiryukov3 years ago

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

Closing in favor of #18465.

comment:2 SergeyBiryukov2 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.

comment:3 nacin2 years ago

  • Version changed from 3.3 to 3.2

comment:4 kurtpayne2 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

comment:5 joostdevalk2 years ago

My opinion? Do them all.

comment:6 follow-up: bpetty20 months 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.

comment:7 follow-up: ericlewis20 months ago

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

comment:8 in reply to: ↑ 7 bpetty20 months 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.

comment:9 in reply to: ↑ 6 ; follow-up: SergeyBiryukov20 months 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 20 months ago by SergeyBiryukov (previous) (diff)

comment:10 in reply to: ↑ 9 bpetty20 months 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.

comment:11 nacin3 months ago

  • Component changed from General to Bootstrap/Load
Note: See TracTickets for help on using tickets.