WordPress.org

Make WordPress Core

Opened 13 months ago

Closed 5 months ago

#24646 closed defect (bug) (fixed)

fetch_feed() returns WP_Error with "A valid URL was not provided"

Reported by: husdaman Owned by: nacin
Milestone: 3.6 Priority: normal
Severity: major Version: 3.5.2
Component: Feeds Keywords: fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Since I upgraded my to 3.52 nothing fetch_feed does not work properly. It gets feeds outside of my network but does not get feeds inside of my network. I don't know why this is happening. I get this error when doing var_dump

object(WP_Error)#89 (2) { ["errors"]=> array(1) { ["simplepie-error"]=> array(1) { [0]=> string(44) "WP HTTP Error: A valid URL was not provided." } } ["error_data"]=> array(0) { } }

When I go manually to the entered feed url, I can see it display fine in the browser so I know the feeds are working.

I also wanted to point out that I tested this on another 3.51 version of a blog on our network and it worked fine but as soon as I updated to 3.52, I got the same error above.

Attachments (6)

24646.diff (6.7 KB) - added by nacin 11 months ago.
Use wp_safe_remote_*() in core.
24646.2.diff (1.7 KB) - added by nacin 11 months ago.
More ways to mark a URL as 'safe'
24646.3.diff (3.6 KB) - added by nacin 11 months ago.
24646.4.diff (3.6 KB) - added by nacin 11 months ago.
24646.5.diff (3.7 KB) - added by nacin 11 months ago.
24646.6.diff (3.4 KB) - added by nacin 11 months ago.

Download all attachments as: .zip

Change History (56)

comment:1 SergeyBiryukov13 months ago

  • Keywords dev-feedback added; reporter-feedback removed
  • Version set to 3.5.2

comment:2 SergeyBiryukov13 months ago

  • Description modified (diff)

comment:3 ocean9013 months ago

What's the URL of the feed?

comment:4 ocean9013 months ago

  • Summary changed from fetch_feed for 3.52 to fetch_feed() returns WP_Error with "A valid URL was not provided"

comment:5 georgestephanis13 months ago

Also, can you give us some details as to your server configuration? OS/Webserver (IIS, Apache, etc)

comment:6 aaroncampbell13 months ago

  • Cc aaroncampbell added

comment:7 ocean9013 months ago

IRC chat about a related issue while network upgrading.

comment:8 husdaman13 months ago

Hi guys, thanks for your replies.

Here is the url: http://datr-web.hwdsb.on.ca/schools/?school=balaclava

Check under the Latest news section near the bottom(Programed to say "News not available" if it fails). In the above example the site feed I am trying to get is schools.hwdsb.on.ca/balaclava/feed which works.

Running on IIS7 with php on windows VM.

Thanks

comment:9 aaroncampbell13 months ago

I actually can't get fetch_feed( 'schools.hwdsb.on.ca/balaclava/feed' ) to work on 3.5.1 or 3.5.2. However, fetch_feed( 'http://schools.hwdsb.on.ca/balaclava/feed' ) worked for me on both. It looks like Simplepie calls SimplePie_Misc::fix_protocol() on all URLs, but if the URL is missing a protocol that method calls itself again passing http as the protocol. On the second call the URL (in my case the slashes) are getting encoded causing the URL to be invalid.

The easy fix for you right now is to pass full URL with the protocol to fetch_feed()

comment:10 aaroncampbell13 months ago

Just to clarify, it's not specific to your URL. Passing in my feed without a protocol (ran.ge/feed) doesn't work but with a protocol it does (http://ran.ge/feed)

comment:11 husdaman13 months ago

Thanks for your reply. Sorry but my codes does include the protocol in the backend but again it does not fetch anything. Anything outside of the hwdsb network works though so I'm very confused as to why this is.

comment:12 husdaman13 months ago

I'm not sure if it's my theme? I don't even know where to look now.

comment:13 nacin13 months ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 3.5.3

It has nothing to do with the theme or, in this case, the URL. Rather, it has to do with the DNS resolution of that URL's host, specifically on your own server.

husdaman, can you please run var_dump( gethostbyname( 'schools.hwdsb.on.ca' ), gethostbyname( ' ​datr-web.hwdsb.on.ca' ) ) from your server?

It looks like we are going to have to improve the check in wp_http_validate_url() (added in 3.5.2) to do more than compare the same host. Any loosening of these restrictions, however, have security implications, so we need to be very careful and very well informed.

comment:14 husdaman13 months ago

Hi,

Okay so I ran var_dump( gethostbyname( 'schools.hwdsb.on.ca' ), gethostbyname( 'datr-web.hwdsb.on.ca' ) );

http://datr-web.hwdsb.on.ca/schools/?school=balaclava shows the result string(12) "10.155.3.164" string(12) "10.155.3.184"

Thanks

comment:15 ocean9013 months ago

See trunk/wp-includes/http.php@24480#L369, private addresses are blocked.

comment:16 follow-up: Otto4213 months ago

Private addresses being blocked is a problem for cases where WordPress is used on internal networks, but just allowing it indiscriminately is potentially a security problem.

One can get around this by using the 'http_request_reject_unsafe_urls' filter, but perhaps a filter more specific to the private-IP range check would be appropriate here, to allow people using it in this way to bypass just that check.

@husdaman: A temporary workaround:

add_filter('http_request_reject_unsafe_urls','__return_false');

This disables the URL safety check entirely, so only use it in cases where the site is not-publicly accessible (like your internal site).

comment:17 faceinthecrowd13 months ago

I have the same trouble with our environment. The problem is that internal DNS resolves our news feed (on a different server) to a private IP address, however to anyone outside our network, the IP will resolve to the public, NAT'd address, which would validate. We have had to disable the reject_unsafe_url check for this to work in our environment, though we do not want to do this for obvious reasons.

I feel like this is a pretty common configuration where internal/external DNS will resolve to different IP addresses and there needs to be a more robust check for these conditions, though I am not experienced enough to help with the resolution.

Will

comment:18 Otto4213 months ago

Another alternative: currently this is checking for the $same_host field and ignoring the private check if the domain of the URL and the domain of the home match. Perhaps extending this to allow base domain matches would fit the bill.

So aaa.example.com and bbb.example.com would be able to talk to each other without the private IP check.

comment:19 follow-up: nacin13 months ago

What about:

var_dump( gethostbyname( 'schools.hwdsb.on.ca.' ), gethostbyname( ' ​datr-web.hwdsb.on.ca.' ) )

comment:20 follow-up: mboynes13 months ago

  • Cc mboynes@… added

I've run into a couple similar issues since updating to the latest nightly build. First off, I had the same issue as @ocean90 when doing a network upgrade. Second, I can no longer query a server on a non-standard port.

I believe this came in with the addition of wp_http_validate_url in 24480. While I always respect a push for additional security, this one might be a little much. Furthermore, if I'm reading it correctly (and please, correct me if I'm wrong), it seems like this was added a week ago, is already live in 3.5.2, and didn't even get a nod in the release announcement. If that's the case, developers really had no warning that this was coming, no notice that it deployed, and no time to test their code. Am I missing something really obvious or am I completely misreading what happened?

comment:21 in reply to: ↑ 20 ; follow-up: nacin13 months ago

Note: There is a difference in functionality between 3.5.2 and trunk. If you are experiencing problems in trunk, please test to see if you are experiencing those problems in 3.5.2. Trunk is more aggressive in rejecting URLs. We need detailed information to figure out how aggressive is too aggressive. Detailed information can also help identify potentially safe situations that we are rejecting as unsafe.

Replying to mboynes:

I can no longer query a server on a non-standard port.

Which port? What kind of request? What protocol? What are you trying to accomplish? What's your site's port? How are you making this query?

You said latest nightly build. Just to be clear — are you having issues on 3.5.2, or only trunk?

I believe this came in with the addition of wp_http_validate_url in [24480].

Yes.

While I always respect a push for additional security, this one might be a little much. Furthermore, if I'm reading it correctly (and please, correct me if I'm wrong), it seems like this was added a week ago, is already live in 3.5.2, and didn't even get a nod in the > release announcement.

The changes to the HTTP API are not enumerated in the announcement post, no, but it is part of the first bullet point. There will be a post soon on API changes.

If that's the case, developers really had no warning that this was coming, no notice that it deployed, and no time to test their code. Am I missing something really obvious or am I completely misreading what happened?

No, that's what happened. WordPress 3.5.2 is, very simply, a security release for all previous WordPress versions.

There was definitely the possibility that [24480] would break functionality. Believe it or not, we decided to break far less than we intended.

Good thing is, 3.6 is just around the corner. We're happy to entertain any detailed information that can help improve wp_http_validate_url().

comment:22 in reply to: ↑ 19 husdaman13 months ago

Replying to nacin:

What about:

var_dump( gethostbyname( 'schools.hwdsb.on.ca.' ), gethostbyname( ' ​datr-web.hwdsb.on.ca.' ) )

string(12) "10.155.3.164" string(12) "10.155.3.184"

comment:23 in reply to: ↑ 21 ; follow-up: mboynes13 months ago

Replying to nacin:

Which port? What kind of request? What protocol? What are you trying to accomplish? What's your site's port? How are you making this query?

I'm querying an elasticsearch server on port 9200. I'm also accessing it through a local IP, as it so happens.

You said latest nightly build. Just to be clear — are you having issues on 3.5.2, or only trunk?

Sorry for the confusion; I'm experiencing this on trunk. I didn't realize that what went live in 3.5.2 and what's in trunk were different. I saw the commit was merged into 3.5.2 and didn't dig any further. I didn't test any of this on 3.5.2.

The changes to the HTTP API are not enumerated in the announcement post, no, but it is part of the first bullet point. There will be a post soon on API changes.

Great, thanks. I think that's really important for developers.

No, that's what happened. WordPress 3.5.2 is, very simply, a security release for all previous WordPress versions.

There was definitely the possibility that [24480] would break functionality. Believe it or not, we decided to break far less than we intended.

Good thing is, 3.6 is just around the corner. We're happy to entertain any detailed information that can help improve wp_http_validate_url().

I'd like to read more about the decision to limit the HTTP API. I couldn't find a ticket on the changes, so if you can point me in the right direction to follow the discussion, I'd really appreciate it. What I'd like to learn more about is, what's the rationale/threat? Why block local IPs? Why block non-standard ports? I'm clearly not evil enough, because I can't see the harm in either.

Thanks!

comment:24 in reply to: ↑ 23 ; follow-up: nacin13 months ago

Replying to mboynes:

I'm querying an elasticsearch server on port 9200. I'm also accessing it through a local IP, as it so happens.

As in, you are making a direct call to wp_remote_request() to 'x.x.x.x:9200'? Code helps us understand.

I didn't test any of this on 3.5.2.

Might you be able to?

I'd like to read more about the decision to limit the HTTP API. I couldn't find a ticket on the changes, so if you can point me in the right direction to follow the discussion, I'd really appreciate it. What I'd like to learn more about is, what's the rationale/threat? Why block local IPs? Why block non-standard ports? I'm clearly not evil enough, because I can't see the harm in either.

http://cwe.mitre.org/data/definitions/918.html

comment:25 in reply to: ↑ 24 mboynes13 months ago

Replying to nacin:

Sorry for the delay. Keeping the code as simple as possible:

print_r( wp_remote_get( 'http://localhost:9200' ) );

This does not work in the latest nightly build (I get "A valid URL was not provided."). I tested it on 3.5.2 as requested and it does continue to work there. Of course, you don't need to be running an elasticsearch server to test this; you'd either get "couldn't connect to host" or "a valid url was not provided" on 3.5.2 and trunk respectively. I tested with non-local hosts, IPs (local and public), and got the same results in all cases (not particularly surprising since the issue is the port, but there's no harm in thorough testing).

comment:26 follow-up: joehoyle12 months ago

I recently ran into this (well, @matthue did!) with local development. When local site "A" wants to send a request to local site "B", it will be rejected, as that resolves to 127.0.0.1 via the hosts file. I think this is the pattern a lot of developers use with vhosts etc, so might be worth consideration if this goes into the next release from trunk.

I don't know the exact security vulnerability and don't need to discuss that here, just wanted to point out (at least my) development process is affected by this change.

Another setup I run on a prod box is 2 sites on the same machine, where they have "127.0.0.1 example.com" in the hosts file, which I believe again would mean wp_http_validate_url would brake communication between the two sites.

comment:27 in reply to: ↑ 26 ; follow-up: nacin12 months ago

Replying to joehoyle:

I recently ran into this (well, @matthue did!) with local development. When local site "A" wants to send a request to local site "B", it will be rejected, as that resolves to 127.0.0.1 via the hosts file. I think this is the pattern a lot of developers use with vhosts etc, so might be worth consideration if this goes into the next release from trunk.

The specific change from 3.5.2 to 3.6 has to do with [24482]. Specifically, the HTTP API will by default reject all URLs it thinks is unsafe, for all requests, not just the ones that explicitly pass an argument as in [24480]. We're not sold on [24482] for 3.6, and rather wanted to "test flight" it to see what kind of bug reports we would get. So thank you for helping!

I think we can safely allow traffic to 127.0.0.1 as long as the existing site's domain also resolves to 127.0.0.1, and the port is standard (80, 443, or 8080, or if the port matches the site's port (which would work for :8888 to :8888). We could also directly whitelist 8888 which is MAMP's default).

Unfortunately WP_DEBUG is not a great indicator of "production" versus "development", so we couldn't just allow unsafe URLs for WP_DEBUG only.

Another setup I run on a prod box is 2 sites on the same machine, where they have "127.0.0.1 example.com" in the hosts file, which I believe again would mean wp_http_validate_url would brake communication between the two sites.

Yes — this definitely has the potential to break that setup as well, which has been reported here. I am still not sure how to address this.

Without going into an incredible amount of detail, we're trying to prevent server-side request forgery attacks. (Imagine me tricking your local server trying to attack your router.) So locally-resolved requests are, in general, untrusted. So we need to figure out how to make some of them "work" without opening ourselves up to a vulnerability.

Even if we change the default (as in, reverting [24482], which is very likely at this time), we'll still need to allow locally-resolved requests to work in certain cases. It appears there are two cases: When a server directly resolves to itself or a nearby server in production via /etc/hosts; and a similar development setup where domains are set to 127.0.0.1.

I'm happy to discuss in deeper detail in private — nacin or security -at- wordpress.org — especially if anyone has some interesting ideas.

comment:28 in reply to: ↑ 27 ; follow-up: aaroncampbell12 months ago

Replying to nacin:

I think we can safely allow traffic to 127.0.0.1 as long as the existing site's domain also resolves to 127.0.0.1, and the port is standard (80, 443, or 8080, or if the port matches the site's port (which would work for :8888 to :8888). We could also directly whitelist 8888 which is MAMP's default).

Seems like it would be OK to allow traffic to ANY internal IP as long as the existing site's domain also resolves to it. That would allow for setups like mine where a few domains on a bank of servers are all pointed to an internal IP for a load balancer.

comment:29 in reply to: ↑ 28 nacin12 months ago

Replying to aaroncampbell:

Seems like it would be OK to allow traffic to ANY internal IP as long as the existing site's domain also resolves to it. That would allow for setups like mine where a few domains on a bank of servers are all pointed to an internal IP for a load balancer.

It's not. The goal of a server-side request forgery is to trick the server into sending an internally routed request when one is not desired. One may sometimes be desired, but there's possibly a lot of internal things that bank of servers can talk to, but shouldn't arbitrarily. Like, say, a memcached server. The best firewalls can lock most of that chatter down, but it's rare — your server will still need to connect to the memcached server, but only on its terms, not the terms of an HTTP request with an arbitrary URL.

comment:30 aaroncampbell12 months ago

That's a good point. I ended up adjusting my config to use an external IP for the load balancer and everything seems to be working fine now anyway.

comment:31 in reply to: ↑ 16 ; follow-up: tjb101312 months ago

Replying to Otto42:

@husdaman: A temporary workaround:

add_filter('http_request_reject_unsafe_urls','__return_false');

This disables the URL safety check entirely, so only use it in cases where the site is not-publicly accessible (like your internal site).

Tried this on my intranet site, but no go.

I pull an RSS feed from a different virtual host that is on the same server. Both sites only accessible internally.

comment:32 in reply to: ↑ 31 ; follow-up: nacin12 months ago

The proper check would instead be:

add_filter( 'http_request_args', function( $args ) {
   $args['reject_unsafe_urls'] = false;
   return $args;
} );

The http_request_reject_unsafe_urls filter only covers defaults.

comment:33 in reply to: ↑ 32 ; follow-up: tjb101312 months ago

Replying to nacin:

The proper check would instead be:

add_filter( 'http_request_args', function( $args ) {
   $args['reject_unsafe_urls'] = false;
   return $args;
} );

The http_request_reject_unsafe_urls filter only covers defaults.

That worked, thanks.

comment:34 in reply to: ↑ 33 ; follow-up: husdaman12 months ago

Replying to tjb1013:

Replying to nacin:

The proper check would instead be:

add_filter( 'http_request_args', function( $args ) {
   $args['reject_unsafe_urls'] = false;
   return $args;
} );

The http_request_reject_unsafe_urls filter only covers defaults.

That worked, thanks.

I'm trying to apply this but keep getting Parse error: syntax error, unexpected T_FUNCTION

Can't figure out where it's occurring. I also put this in my function.php file correct?

I also wanted to point out that I've imported a site from our hwdsb network to a multisite setup and it does not import media anymore. Do you think this is related? Basically I get "Failed to import Media “kg1”" etc.. When I check the media section, nothing is there. When I check the appropriate directory, nothing is there either.

comment:35 in reply to: ↑ 34 ; follow-up: nacin12 months ago

Replying to husdaman:

Replying to tjb1013:

Replying to nacin:

The proper check would instead be:

add_filter( 'http_request_args', function( $args ) {
   $args['reject_unsafe_urls'] = false;
   return $args;
} );

The http_request_reject_unsafe_urls filter only covers defaults.

That worked, thanks.

I'm trying to apply this but keep getting Parse error: syntax error, unexpected T_FUNCTION

You're running PHP 5.2. Please update to a newer version, like 5.3 or 5.4. 5.5 was also just released last month. Or, rewrite the code to work in PHP 5.2.

comment:36 in reply to: ↑ 35 ; follow-up: husdaman12 months ago

Replying to nacin:

Replying to husdaman:

Replying to tjb1013:

Replying to nacin:

The proper check would instead be:

add_filter( 'http_request_args', function( $args ) {
   $args['reject_unsafe_urls'] = false;
   return $args;
} );

The http_request_reject_unsafe_urls filter only covers defaults.

That worked, thanks.

I'm trying to apply this but keep getting Parse error: syntax error, unexpected T_FUNCTION

You're running PHP 5.2. Please update to a newer version, like 5.3 or 5.4. 5.5 was also just released last month. Or, rewrite the code to work in PHP 5.2.

You are right, thank you, I do need to update my development server. On production it was fine. I will retest the import to see if it also fixed the problem.

comment:37 in reply to: ↑ 36 husdaman12 months ago

Replying to husdaman:

Replying to nacin:

Replying to husdaman:

Replying to tjb1013:

Replying to nacin:

The proper check would instead be:

add_filter( 'http_request_args', function( $args ) {
   $args['reject_unsafe_urls'] = false;
   return $args;
} );

The http_request_reject_unsafe_urls filter only covers defaults.

That worked, thanks.

I'm trying to apply this but keep getting Parse error: syntax error, unexpected T_FUNCTION

You're running PHP 5.2. Please update to a newer version, like 5.3 or 5.4. 5.5 was also just released last month. Or, rewrite the code to work in PHP 5.2.

You are right, thank you, I do need to update my development server. On production it was fine. I will retest the import to see if it also fixed the problem.

Alright the import media problem is also gone with the filter fix.

comment:38 nacin12 months ago

#24823 was marked as a duplicate.

comment:39 bmzero12 months ago

We are running an older version of PHP (v 5.2.17). The above code was rewritten to run on our version and placed in our functions.php file.

function allow_unsafe_urls ( $args ) {
	   $args['reject_unsafe_urls'] = false;
	   return $args;
	} ;

add_filter( 'http_request_args', 'allow_unsafe_urls' );

This fixes the issue for our system.

comment:40 nacin11 months ago

In 24894:

Introduce wp_safe_remote_request(). Also wp_safe_remote_head(), wp_safe_remote_get(), wp_safe_remote_post().

Reverts [24482].

see #24646.

comment:41 nacin11 months ago

In 24895:

Add missing documentation from [24894]. see #24646.

comment:42 nacin11 months ago

In 24896:

Introduce wp_safe_remote_request(). Also wp_safe_remote_head(), wp_safe_remote_get(), wp_safe_remote_post().

Reverts [24482].

Merges [24894] and [24895] to the 3.6 branch.

see #24646.

nacin11 months ago

Use wp_safe_remote_*() in core.

nacin11 months ago

More ways to mark a URL as 'safe'

nacin11 months ago

nacin11 months ago

comment:43 nacin11 months ago

Note that [24896] reverted [24482] for 3.6, which should alleviate any issues people were having in 3.6 but *not* in 3.5.2.

To fix some issues in WordPress 3.5.2, I am currently toying with 24646.4.diff. If it detects a host as local, it then does the following:

  • If multisite, it confirms the domain is *not* another site in the multisite install. If it is, then the host is considered "safe".
  • Uses wp_validate_redirect(), which implements an allowed_redirect_hosts filter, which is used in wp_safe_redirect(). If we are safe to publicly redirect to a page, we're also safe to send an HTTP request to it — so we let these through.
  • Adds a http_request_host_is_external filter. Hopefully the name is obscure and specific enough to not cause people to randomly whitelist everything using this method. In the future, we may wish to block "localhost" from being a safe host even *with* that filter. (If the site is running on "localhost", then it'll already be whitelisted.)

That multisite query hits the domain index in $wpdb->blogs which means it is ridiculously fast. (rboren tried it on WP.com.) Which is why wp_is_large_network() isn't checked. This also only fires when we think it is a locally resolved domain, which on a well-configured server isn't actually the case.

nacin11 months ago

nacin11 months ago

comment:44 nacin11 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 24915:

Additional checks when evaluating the safety of an HTTP request, to avoid false negatives.

  • Check if the host is considered a safe redirect host.
  • Check if the host is another domain in a multisite installation.
  • Add a filter to control this.

This only occurs when the DNS resolution of a domain points elsewhere in an internal network, but only internally (and has its own public IP outside the network). This could be considered a bad configuration.

fixes #24646.

comment:45 nacin11 months ago

In 24916:

Additional checks when evaluating the safety of an HTTP request, to avoid false negatives.

Merges [24915] to the 3.6 branch.
fixes #24646.

comment:46 nacin11 months ago

In 24917:

Use wp_safe_remote_request() and friends instead of reject_unsafe_urls = true.

fixes #24646.

comment:47 nacin11 months ago

In 24918:

Use wp_safe_remote_request() and friends instead of reject_unsafe_urls = true.

Merges [24917] to the 3.6 branch.
fixes #24646.

comment:48 nacin11 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

With [24915] and [24917] ([24916] and [24918] for 3.6), marking this as fixed for 3.6.

Please open new tickets for any further issues. This ticket is only remaining open to backport to the 3.5 branch.

comment:49 stefwilliams7 months ago

I've been having this same problem, trying to load a local RSS feed using wp_widget_rss_output in a mu_plugin, but I can't get wp_safe_remote_* to work...

I specifically pass in the $args with 'reject_unsafe_urls' as false, but when it gets round to running the request in WP_Http, an error_log of the args are coming out with 'reject_unsafe_urls' as true, so it fails...

Can you give an example of how to use wp_safe_remote_* properly? Or do I pass the arguments into wp_widget_rss_output somehow?

comment:50 nacin5 months ago

  • Milestone changed from 3.5.3 to 3.6
  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.