WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 2 weeks ago

#35835 closed defect (bug) (fixed)

_deprecated_file undefined in rss-functions.php

Reported by: thib3113 Owned by: whyisjake
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.5
Component: General Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

If you call wp-includes/rss-functions.php directly, you have an error .

Because _deprecated_file is not defined, you can patch with :

<?php
/**
 * Deprecated. Use rss.php instead.
 *
 * @package WordPress
 */

if(defined(ABSPATH)){
    _deprecated_file( basename(__FILE__), '2.1', WPINC . '/rss.php' );
    require_once( ABSPATH . WPINC . '/rss.php' );
}

Attachments (1)

35835.diff (426 bytes) - added by whyisjake 8 months ago.

Download all attachments as: .zip

Change History (22)

#1 follow-up: @dd32
5 years ago

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

Production sites shouldn't be running with display_errors turned on, so the undefined constants & undefined functions warnings/notices/errors aren't seen as a bugs within WordPress.

We also cover this in our security handbook.

#2 @thib3113
5 years ago

Yes, but visiting this webpage produce a http 500 error. So yes, it's not visible, but it exist ...

Why don't just add a single ligne for stop this error ? And making the same in other not-standalone page ...

#3 follow-up: @dd32
5 years ago

Yes, but visiting this webpage produce a http 500 error.

On a URL which should never be linked to (And you should also have directory indexing disabled ideally).

You'll get a similar error if you access wp-includes/default-filters.php or any number of others too potentially.

#4 in reply to: ↑ 3 @thib3113
5 years ago

Replying to dd32:

Yes, but visiting this webpage produce a http 500 error.

On a URL which should never be linked to (And you should also have directory indexing disabled ideally).

It's is not linked, but pentest software know this addresses and check for path. You'are right, in production, we will set debug to false, but in my opinion, we can make attack with this error, maybe with reloading lot of time, and log lot of errors ( I don't know, i'm not a professional in pentesting )...
I just think, let an error is not a good thing, mostly if you just add one line for remove this ...

if(!defined(ABSPATH)) exit();

#5 in reply to: ↑ 1 @TheGP
4 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to dd32:

Production sites shouldn't be running with display_errors turned on, so the undefined constants & undefined functions warnings/notices/errors aren't seen as a bugs within WordPress.

We also cover this in our security handbook.

That's stupid. I have error notification alerts, and I had alerts about this fatal error too. So I just blocked all errors from Wordpress. And this not improving security, because I can't see any errors from Wordpress right now. So fix it and dont make excuses.

+ it adds a lot of trash data to log files (i have thousands of these errors)
+ bots use this bugs in wordpress to scan websites with display_errors On

No wonder wordpress code developers considered as the worst in the industry))

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

#6 @dd32
4 years ago

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

Remarking as invalid. Discussion can continue; a committer can re-open if they feel the need to.

#7 @SergeyBiryukov
15 months ago

#47945 was marked as a duplicate.

#8 @flymike
15 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Type changed from defect (bug) to enhancement

Some hacker has discovered many of the WordPress files containing calls to _deprecated_file() and is inundating my server with direct GET requests to them.
Because that function is not defined in WordPress, Apache returns status 500 and - because, as an administrator, I want to be informed of status 500 - my inbox is deluged with alerts.
I would block the originating IPs but they'e all different, so coming from spambots. And the advantage to the hacker eludes me completely - but it is what it is, and I have to deal with it.
Couldn't WordPress handle calls to deprecated files/functions a little more elegantly? Like it does with direct calls to other files which should not be accessed directly - with status 200 and zero bytes?

#9 @SergeyBiryukov
15 months ago

  • Milestone set to Awaiting Review

Related: #47154

#10 @zodiac1978
9 months ago

I am not sure why this is handled in this way but the PHP updates in a complete other way.

We do not want to annoy the user with technical details about PHP updates, but we try to make them aware that there is something to fix. We should do the same here at least.

I am not sure why the mentioned line is not added to WordPress to all files which are in need of this, but if we are going this route, why not inform the user about the risk of an enabled display_errors setting?

Therefore I have added an issue to the health check plugin about adding this check:
https://github.com/WordPress/health-check/issues/370

If you are searching for this error message you get roughly 30k hits and these are just those websites which also have enabled public accessible error logging (Google finds the error log file with this error message in it). There will be much more websites with this problem.

@whyisjake
8 months ago

#11 @whyisjake
8 months ago

  • Keywords has-patch 2nd-opinion added
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to whyisjake
  • Status changed from reopened to assigned
  • Version changed from 4.4.2 to trunk

#12 @williampatton
3 weeks ago

Just noting here that I've also ran into bots hitting this file on a website that's high traffic and it causes errors to be logged resulting in alert notifications being sent when they shouldn't be.

If the solution is as simple as bailing early or making sure the function is available before accessing it can that be added to all the deprecated files?

Last edited 3 weeks ago by williampatton (previous) (diff)

#13 @williampatton
3 weeks ago

Patch 35835.diff from @whyisjake solves this issue on this file for me.

Is anything else blocking having this merged and released?

#14 @whyisjake
3 weeks ago

  • Type changed from enhancement to defect (bug)

#15 @whyisjake
3 weeks ago

Yeah, I agree with @williampatton, this is basically a bug that should have been addressed long ago.

#16 @whyisjake
3 weeks ago

  • Milestone changed from Future Release to 5.6

#17 @whyisjake
3 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 49619:

Feeds: Add an early exit when calling RSS functions directly.

While this file is depracated, it shouldn't throw errors when called directly.

Fixes #35835.

Props thib3113, dd32, TheGP, flymike, zodiac1978, williampatton.

#18 follow-ups: @SergeyBiryukov
3 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

There are ~30 files in core with _deprecated_file(), why only this one is being patched? Is there anything specific calling this file directly, but not the others?

Not arguing for or against the change, just for a consistent policy.

This is also not limited to _deprecated_file(), as noted in comment:3 and comment:1:

You'll get a similar error if you access wp-includes/default-filters.php or any number of others too potentially.

Should we try to address other instances too and edit the security handbook accordingly?

#19 in reply to: ↑ 18 @williampatton
3 weeks ago

Replying to SergeyBiryukov:

There are ~30 files in core with _deprecated_file(), why only this one is being patched? Is there anything specific calling this file directly, but not the others?

In my case it was a fuzzing bot that was calling the file directly. I presume to get a file inclusion path disclosure of some kind but I can't be sure of the motives.

Should we try to address other instances too and edit the security handbook accordingly?

I agree that we should try and address other instances too if they can easily be identified. Today I only had this one file in my logs as being accessed directly but in the past there have been some others (I recall one being from ID3 library but I am certain there have been more over time).

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


2 weeks ago

#21 in reply to: ↑ 18 @SergeyBiryukov
2 weeks ago

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

Created #51806 for other instances, re-closing this one as fixed for 5.6.

Note: See TracTickets for help on using tickets.