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

[Nikola Nikolov]

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
>