[wp-hackers] Are there any implications about using wp_redirect( add_query_arg( '', '' ) )?

[J.D. Grimes]

You are right to question whether there might be some issues with this. There is in fact a potential for security issues when using wp_redirect(), but it has nothing to do with escaping the URL. I think it's not actually necessary to use esc_url_raw(), since wp_redirect() uses wp_sanitize_redirect() to sanitize the URL (https://developer.wordpress.org/reference/functions/wp_sanitize_redirect/ <https://developer.wordpress.org/reference/functions/wp_sanitize_redirect/>).

The issues that you would run into with wp_redirect() would be an open redirect (https://www.owasp.org/index.php/Open_redirect <https://www.owasp.org/index.php/Open_redirect>). To avoid that, you can use wp_safe_redirect() (https://developer.wordpress.org/reference/functions/wp_safe_redirect/ <https://developer.wordpress.org/reference/functions/wp_safe_redirect/>) which ensures that the redirect will only go to a host that is on the whitelist. I always use wp_safe_redirect(), just to be sure that the redirect is secure.

As for this case when using add_query_arg(), it shouldn't technically be necessary to use wp_safe_redirect(), since add_query_arg() uses the REQUEST_URI server var, which should be secure. There is always the possibility of course that some other code has fiddled with the REQUEST_URI, and in that case it may not actually end up pointing to the correct host like it should. So that's why I'd recommend that you use wp_safe_redirect(), since you know that you will always be wanting to redirect back to the same site. wp_redirect() should only be used when you specifically want to redirect to a different host, and then you should be hard-coding the URL.

As far as passing empty values to add_query_arg(), that is perfectly valid, although you might as well use $_SERVER['REQUEST_URI'] directly, instead of doing extra, unnecessary processing on the URL. There is also the wp_get_referer() function (https://developer.wordpress.org/reference/functions/wp_get_referer/ <https://developer.wordpress.org/reference/functions/wp_get_referer/>) that you could use if you want to accept more ways of determining the referrer.

-J.D.

> On Feb 16, 2016, at 8:17 AM, Nikola Nikolov <nikolov.tmw at gmail.com> wrote:
> 
> Well, it actually works fine with the example I gave.
> 
> Here, say I'm on http://example.com/page1/?a=b&c=d
> Then I call add_query_arg( '', '' ) - omitting the last parameter, which
> makes it use the current request as the base URL.
> 
> This returns something like this: /page1/?a=b&c=d&
> That last & is there because I tried to add an empty parameter, so there's
> no key, but there's a trailing &(which should not be a problem as far as
> I'm aware).
> 
> My question wasn't about how to use add_query_arg() as much as it was about
> whether there could be any issues by passing two empty parameters to the
> function, since it technically expects those to be strings.
> 
> On Tue, 16 Feb 2016 at 14:57 James DiGioia <jamesorodig at gmail.com> wrote:
> 
>> add_query_args takes two or three parameters, depending on how you'd prefer
>> to use it. See here:
>> https://developer.wordpress.org/reference/functions/add_query_arg/
>> 
>> It's not automatically going to redirect using the current $_GET params;
>> you need to specify in the function args what those params are supposed to
>> be.
>> 
>> On Tue, Feb 16, 2016 at 5:23 AM, Nikola Nikolov <nikolov.tmw at gmail.com>
>> wrote:
>> 
>>> Hi there,
>>> 
>>> I've found myself in situations where I want to reload the current URL
>> with
>>> PHP - for instance after processing a $_POST request - and in some cases
>> I
>>> would add a $_GET parameter to the URL(so add_query_arg() would be a
>>> perfect fit there), but in others I don't need to do that. I found out
>> that
>>> I can use
>>> 
>>> wp_redirect( esc_url_raw( add_query_arg( '', '' ) ) );
>>> exit;
>>> 
>>> Used that way add_query_arg() seems to just give me a path and any $_GET
>>> parameters currently present(for instance I would get
>>> /product/my-product/). I'm escaping the URL with esc_url_raw(), but
>> wanted
>>> to make sure there's nothing wrong with doing things that way.
>>> 
>>> Any thoughts?
>>> _______________________________________________
>>> wp-hackers mailing list
>>> wp-hackers at lists.automattic.com
>>> http://lists.automattic.com/mailman/listinfo/wp-hackers
>>> 
>> _______________________________________________
>> wp-hackers mailing list
>> wp-hackers at lists.automattic.com
>> http://lists.automattic.com/mailman/listinfo/wp-hackers
>> 
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers