Audit use of wp_redirect

[benhuson]

Check wether we should be using wp_safe_redirect() anywhere to enforce redirects to same (or allowed) domain only. http://codex.wordpress.org/Function_Reference/wp_safe_redirect

Helps to prevent open redirect vulnerability https://www.owasp.org/index.php/Open_redirect

Just had this pointed out to me in one of my plugins so thought is was worth checking.

[benhuson]

These are the only uses of wp_redirect I could find worth checking:

I'm guessing these are OK using $_SERVER['REQUEST_URI']? https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/display-sales-logs.php#L306 https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/includes/product-variations-page.class.php#L301 https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/includes/product-variations-page.class.php#L345 https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-core/wpsc-functions.php#L1511

During image generation should it be used? Once generated I guess not as it could exist on a CDN? https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-includes/ajax.functions.php#L1031

Payment Gateway redirects should be left as is I guess.

[JustinSainton]

Yeah, WP Core is pretty consistent about wp_redirect() usage with $_SERVER['REQUEST_URI']. So I think we're alright there.

Where you see wp_safe_redirect()'s is quite consistently when there are POSTed referrers being redirected to. So if we're using POSTed referers or wp_get_referer(), I think those would be fine times to use wp_safe_redirect(). You want to re-audit this for 3.8.12? My guess is we likely won't need to change anything, but it's possible.

[benhuson]

I've done another search excluding instances where it's used with $_SERVER['REQUEST_URI'].

On the Codex, there is an example that shows use of wp_safe_redirect () when using wp_get_referer() as I think wp_get_referer() can accept external URLs? http://codex.wordpress.org/Function_Reference/wp_get_referer#Examples

I could find the following instances where this is true for us.
I've ignored instances where check_admin_referer() is called previously in the code as that should valid the the referrer, I guess? I'm not that familiar with this admin code to check wether wp_safe_redirect() is appropriate whichout tracing back the specific functions to find when/how that are used.

Check off the instances below which have been checked:

• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/ajax-and-init.php#L212
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/display-items.page.php#L570
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L81
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L287
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L325
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L363
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L428
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L512
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L543
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L616
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L688
• [ ] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-admin/init.php#L700
• [x] https://github.com/wp-e-commerce/WP-e-Commerce/blob/master/wpsc-includes/theming.class.php#L64

[JustinSainton]

As of 3.6, (our minimum is 3.7), this is less of an issue. For the exact reasons you mention (wp_get_referer() accepting external references), wp_redirect() has been shored up in core (Context: Trac ticket).

We should definitely fix this, but it's a minor issue now, more than a major issue.