Security: Recommend wp_safe_remote_*() when wp_remote_*() is used

[JDGrimes]

The wp_remote_*() functions (like wp_remote_get()) do not validate the passed URL, resulting in the possibility for malicious requests if the $url is user-controlled. The wp_safe_remote_*() functions should be instead, since "the URL is validated to avoid redirection and request forgery attacks."

[jrfnl]

👍
Couple of notes/questions:

• Not all wp_remote_*() functions have a wp_safe_remote() counterpart.
  So should the check only cover those methods which do have a counterpart ?
  Refs:
  • https://developer.wordpress.org/?s=wp_remote (13 results)
  • https://developer.wordpress.org/?s=wp_safe_remote (4 results)
• What if the intention is an external redirect ?
  Looking at the function trace/code, my guess would be that wp_safe_remote_get() should still be used, but combined with the http_request_host_is_external filter. Is that correct ?
  If so, extra care will need to be taken to craft the warning in a way that it is directly actionable.
• Both the VIP.RestrictedFunctions as well as the WP.AlternativeFunctions sniff already contain some checks for wp_remote_*(). Those may need to be removed/adjusted when this issue gets addressed.
• The underlying function which is used to validate the URL is wp_http_validate_url().
  What about if that function is already used before wp_remote_*() is called ?

[JDGrimes]

Not all wp_remote_*() functions have a wp_safe_remote() counterpart.

I had forgotten about those. The wp_remote_retrieve_*() functions don't need a counterpart, because they actually just get passed the response and return a particular part of it.

And it looks like wp_remote_fopen() actually uses wp_safe_remote_get(), so it should be OK too.

So just the ones that the safe version exists for actually need it.

What if the intention is an external redirect ?
Looking at the function trace/code, my guess would be that wp_safe_remote_get() should still be used, but combined with the http_request_host_is_external filter. Is that correct ?

Actually, the concern of that filter is internal redirects, not external ones. Or rather, redirects to internal URIs. The issue is SSRF, I think. (Similar to CSRF.) That filter would be used to specifically allow access to certain internal resources that WordPress would usually block requests to. The reason that such requests would normally be dangerous is that the server obviously has the ability to access many internal resources that usually aren't made available to external requests. But if the $url is under an attacker's control, then they suddenly have the ability to make requests to those internal resources as well, because the server is making the requests on the attacker's behalf. Thus, request forgery.

The underlying function which is used to validate the URL is wp_http_validate_url().
What about if that function is already used before wp_remote_*() is called ?

Personally, my response is always, "And what if it isn't?" I just stick with the safe versions of the functions and that way it is always obvious if a request is protected or not.

[jrfnl]

Actually, the concern of that filter is internal redirects, not external ones.

Ok, guess I'll have to have a play with the _safe_* functions a little more at some point.