WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#17601 closed defect (bug) (invalid)

Fatal Errors at URLs Such As canonical.php

Reported by: miqrogroove Owned by:
Milestone: Priority: normal
Severity: trivial Version:
Component: General Keywords:
Focuses: Cc:

Description

Unless there is a documented best practice saying the wp-includes directory should be firewalled during installation, some attention needs to be given to the fatal errors that are being generated by these scripts in production environments.

For example,

PHP Fatal error: Call to undefined function add_action() in wp-includes/canonical.php on line 343

Change History (21)

comment:1 in reply to: ↑ description ; follow-up: @nacin4 years ago

Replying to miqrogroove:

Unless there is a documented best practice saying the wp-includes directory should be firewalled during installation

display_errors should be off, rather.

comment:2 in reply to: ↑ 1 @miqrogroove4 years ago

Replying to nacin:

display_errors should be off, rather.

The reported fatal error occurs just fine with display_errors turned off. Did you mean error_reporting should be zero?

comment:3 @nacin4 years ago

Sorry, I thought you were reporting path disclosure.

comment:4 @miqrogroove4 years ago

Nope :) I'm literally just reporting some unexpected errors being logged.

comment:5 @scribu4 years ago

  • Keywords reporter-feedback added

Are you sure this isn't caused by a plugin or custom theme?

Also, what WP version are you using?

comment:6 @scribu4 years ago

  • Keywords reporter-feedback removed

Oh, you mean when you access wp-includes/canonical.php directly in your browser.

comment:7 @scribu4 years ago

  • Severity changed from normal to trivial

comment:8 @miqrogroove4 years ago

See also

class-snoopy.php
default-embeds.php
default-filters.php
default-widgets.php
feed-atom.php
feed-atom-comments.php
feed-rdf.php
feed-rss.php
feed-rss2.php
feed-rss2-comments.php
ms-default-constants.php
ms-default-filters.php
ms-settings.php
nav-menu-template.php
registration-functions.php
rss.php
rss-functions.php
script-loader.php
shortcodes.php
template-loader.php
update.php
vars.php
wp-db.php

comment:9 @miqrogroove4 years ago

And in wp-admin/includes/

admin.php
comment.php
continents-cities.php
media.php
nav-menu.php
schema.php
update.php
upgrade.php
user.php

comment:10 @azaozz4 years ago

There was an (quite) old ticket about adding

if ( !defined('ABSPATH') )
  die;

to the top of all files that are not a point of entry. Think it was decided it's not worth it as this can result in errors only when WordPress is uploaded to the server but not installed, i.e. for a very short period of time.

comment:11 @miqrogroove4 years ago

For wp-admin/includes/ I think I can deny from all, but the wp-includes structure seems more complex. It has some subdirectories with server-side scripts and some with client-side. Not really sure what the best way is to proceed without researching what all of those files are supposed to do.

comment:12 @miqrogroove4 years ago

Something like this perhaps:

wp-admin/includes/*
wp-includes/*.php
wp-includes/js/tinymce/langs/wp-langs.php
wp-includes/theme-compat/*

comment:13 @miqrogroove4 years ago

Works perfectly in the root .htaccess file:

RewriteRule ^wp-admin/includes/ - [F,L]
RewriteRule !^wp-includes/ - [S=3]
RewriteRule ^wp-includes/[^/]+\.php$ - [F,L]
RewriteRule ^wp-includes/js/tinymce/langs/.+\.php - [F,L]
RewriteRule ^wp-includes/theme-compat/ - [F,L]

comment:14 @miqrogroove4 years ago

Plugins, oy!

comment:15 @dd324 years ago

as azaozz mentions, there was a previous ticket for this, closed as wontfix/invalid.

The reason put forward for adding it was to prevent the "security problem" of path disclosure. From memory, it was decided that path disclosure was not a classified as a security vulnerability to WordPress.

The other reason mentioned was showing errors to users, This was ignored too, citing the fact that it's poor server setup to have display errors on on a production server.

Issue at hand is the errors being logged, but the real question is, why are the files being hit in the first place? That shouldn't happen, links will never be generated to the files. /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.

So the only problem I see here at all, is wp-includes being indexed.

comment:16 @nacin4 years ago

As long as the goal is unnecessary crawling, I'm fine with an index.php in wp-includes and wp-admin/includes. Any other worthy directories?

comment:17 @miqrogroove4 years ago

I disagree with unnecessary crawling being the issue here. Directory listing was disabled and /wp-includes was denied in robots.txt when this problem came to my attention.

comment:18 @nacin4 years ago

This isn't really something I feel we should worry ourselves over. Configuration at the server level is surely enough?

comment:19 @miqrogroove4 years ago

I feel like we could at least put my .htaccess snippet on a Codex page somewhere for people who don't want WordPress to generate fatal errors.

comment:21 @westi4 years ago

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

Closing as invalid as this is not a bug.

The htaccess in the codex seems a suitable thing to do.

Note: See TracTickets for help on using tickets.