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

[J.D. Grimes]

I'm not sure if a Location header would be vulnerable to an unescaped URL or not. The most common reason to escape add_qeury_arg() is to avoid XSS.

-J.D.

> On Feb 16, 2016, at 10:16 AM, Nikola Nikolov <nikolov.tmw at gmail.com> wrote:
> 
> I used esc_url_raw() because I did read somewhere in the comments about the
> security issue of not escaping add_query_arg() with a missing parameter for
> the base URL. However the case they were discussing then might have been if
> you want to use it in a Location header(can't remember).
> 
> Good point on wp_safe_redirect() - even though you'll most-likely end up on
> the same site, it's a good rule of thumb to use that when the destination
> is expected to be the same site.
> 
> Thank you for the detailed explanation!
> 
> On Tue, 16 Feb 2016 at 16:25 J.D. Grimes <jdg at codesymphony.co> wrote:
> 
>> 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
>> 
>> _______________________________________________
>> 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