WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 3 months ago

#8910 reopened defect (bug)

Limit the RSS widget from using its own url

Reported by: webmaestro Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8
Component: Widgets Keywords: needs-patch close
Focuses: Cc:

Description

NOTE: Copied from http://trac.mu.wordpress.org/ticket/852 since 'This is really a WP issue, not strictly MU. Can you open a ticket in WP's trac?'

I admit that this is not a well conceived thought but I wanted to throw it out.

There's some discussion on the premium site about individual blogs using their rss widgets to display their own feeds. This of course causes issues with an unnecessary rss feed pull as well as old data since the feeds are cached.

May I suggest a check to see if the inputted RSS feed matches the individual blog's own rss feed, kick it out with an error and suggesting to the user that they use the Latest Posts widget instead?

Marking this as 2.8 as it's not a high priority.

Change History (15)

comment:1 ryan5 years ago

  • Component changed from Feeds to Widgets

comment:2 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added

comment:3 Denis-de-Bernardy5 years ago

  • Milestone changed from 2.8 to Future Release

punting from lack of patch

comment:4 Viper007Bond5 years ago

  • Priority changed from normal to lowest
  • Severity changed from normal to minor

comment:5 Denis-de-Bernardy5 years ago

  • Milestone Future Release deleted
  • Resolution set to invalid
  • Status changed from new to closed

I think this should be done in WPMU only. Else, you cannot properly guess whether a feed is local or not.

If you've a WP install in /, and another in /folder/, then an rss widget in / that grabs /folder/feed/ would be misled into thinking it needs to get a local feed -- and it would get a 404 error.

comment:6 casabsolute5 years ago

  • Cc casabsolute added
  • Keywords rss widget url HTTP/1.0 server loop feed recent posts archive added
  • Resolution invalid deleted
  • Severity changed from minor to major
  • Status changed from closed to reopened

Apologies - Reopening because we had a major server issue/database corruption related directly to this bug and was told at the Wordpress MU forums that this is a Wordpress issue, not strictly MU.

Check this post here for further info - http://mu.wordpress.org/forums/topic/9431?replies=8#post-76328

Our server was getting slammed with GET / HTTP/1.0 requests over and over and over - several per second - rampant unneccessary usage of CPU cycles.

We finally tracked the issue down to the built-in RSS widget -

If blog address = http://myblog.funblog.com and feed is at = http://myblog.funblog.com/feed

If using the RSS widget to display the most recent posts from the blog it is activated on, user MUST input the feed address "EXACTLY" as above.

Adding a trailing slash - such as = http://myblog.funblog.com/feed/ - OR - not adding /feed - such as = http://myblog.funblog.com - inside the widget causes the server to go into an infinite loop when someone brings up that blog in their browser window.

In our case, it also corrupted database tables and filled the server logs/space on the server until it was tracked down and resolved.

Possible resolutions -

  1. Add a check into the built-in RSS widget to not allow invalid URLs (those with a trailing slash or without /feed at the end).
  1. Make it clearer (stupid-proof) that the built-in RSS widget is to be used for OUTSIDE feeds, and that the Archive widget can be used to display Recent Posts from the blog you are adding it to (same end product the user is probably looking for in reaching for the RSS widget first, but doesn't realize it's just that easy and is over-thinking it).

comment:7 Denis-de-Bernardy5 years ago

  • Milestone set to Future Release

comment:8 Denis-de-Bernardy5 years ago

  • Type changed from defect (bug) to enhancement

comment:9 uglyrobot4 years ago

  • Priority changed from lowest to normal
  • Type changed from enhancement to defect (bug)

comment:10 dd324 years ago

First up, Disallowing local URL's into the RSS widget is likely to cause issue. there is nothing wrong with using a local url, but as with remote url's, if its incorrect, you can have issues.

I know people who use the RSS widget to display their own feed for 1 simple reason, The styling and HTML output is the same as that of their other feeds they have in that sidebar, It can go along way to easing styling if everything is the same.

So, If there needs to be some extra catches in the RSS widget, like setting the max-redirects, or validating that the url returns a 200 only when set, Then lets do it.

comment:11 robechar4 years ago

  • Priority changed from normal to high
  • Version changed from 2.8 to 2.9.2

From the perspective of systems team at our university, this is a critical security issue. Our team uses Wordpress MU for our university blogging system, and on four separate occasions we have had members set their RSS widget to their own RSS feed. In our system, the loop results in a huge number of open processes, and effectively takes down one of our servers.

The ability for any member to take out a server is a major security issue for us, and we have had a lot of pressure to move away from Wordpress for this reason. We have written a patch which disallows users from selecting their own RSS feed, but we would like to see this issue addressed at the core level, so we don't have to patch every release, and to prevent this issue from being exploited on other systems.

While theoretically including your own RSS feed is a valid thing to do, as dd32 points out, the architecture of Wordpress currently doesn't handle it properly. In the short-term, until that issue can be fully addressed, adding this patch to core should prevent this issue from being exploited.

Patch for /wp-includes/default-widgets.php

741,759d740
<     /**
<      * Override save settings to prevent inclusion of your own RSS feed
<      *
<      * Written in response to this issue
<      * http://core.trac.wordpress.org/ticket/8910
<      */
<     function save_settings($settings) {
<         global $current_blog;
<         // Make sure this blog isn't including their own RSS feed
<         foreach ($settings as $index=>$fields) {
<             if (!empty($fields['url'])) {
<                 if (substr_count($fields['url'], $current_blog->domain . $current_blog->path)) {
<                     $settings[$index]['url'] = '';
<                 }
<             }
<         }
<           return parent::save_settings($settings);
<       }
<

comment:12 westi4 years ago

  • Cc westi added
  • Version changed from 2.9.2 to 2.8

I don't think blocking local feed requests is the right thing to do here.

I think the likely root cause of your current issue is #11585 - the feed fetching is failing and we aren't caching that so every page load triggers another request back onto the server which because the url is wrong for the feed and you are then likely serving a 404 with the feed widget on the page this then generates another request ...

Solving #11585 is the long term solution for this.

For now I would recommend you run with a mu-plugin which disabled the default RSS widget and implements a replacement one with the above rule it in to make it easy for you to upgrade when a new version is released.

Resetting Version to original version the issue was reported in.

comment:13 nacin3 years ago

(In [17260]) Disallow a self-reference on RSS widget save, which would DoS a site. Checking home/siteurl should cover the vast majority of cases. see #8910.

comment:14 solarissmoke2 years ago

  • Keywords rss widget url HTTP/1.0 server loop feed recent posts archive removed

comment:15 bpetty3 months ago

  • Keywords close added
  • Priority changed from high to normal
  • Severity changed from major to normal

Is this technically resolved with r17260? It seems like it is.

Note: See TracTickets for help on using tickets.