WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#4606 closed defect (bug) (fixed)

Redirection Vulnerability in wp-pass.php

Reported by: snakefoot Owned by: markjaquith
Milestone: 2.0.12 Priority: high
Severity: major Version: 2.0.10
Component: Security Keywords: developer-feedback has-patch security redirect
Focuses: Cc:

Description

It is possible to create an url to a trusted Wordpress blog, that redirects to an evil site:

http://vulnerable.blog/wordpress/wp-pass.php?_wp_http_referer=http://www.evilsite.com

http://blogsecurity.net/wordpress/news-050707/

Attachments (4)

4606.patch (687 bytes) - added by hakre 7 years ago.
Patch to solve remote 302 redirect injection flaw #4606
4606.002.diff (1.8 KB) - added by markjaquith 7 years ago.
introducing wp_safe_redirect()
wp2-4606.002.diff (1.9 KB) - added by snakefoot 7 years ago.
Fix for branch 2.0
wp2-4606.003.diff (3.2 KB) - added by snakefoot 7 years ago.
Fix for branch 2.0 (With host filter)

Download all attachments as: .zip

Change History (18)

comment:1 follow-up: matt7 years ago

This is known and will be addresses in 2.2.2 when it is released, however it is not critical enough to necessitate a release fire drill.

comment:2 foolswisdom7 years ago

  • Milestone changed from 2.2.2 to 2.2.3

comment:3 in reply to: ↑ 1 Nazgul7 years ago

  • Keywords developer-feedback added

Replying to matt:

This is known and will be addresses in 2.2.2 when it is released, however it is not critical enough to necessitate a release fire drill.

Was this included in the 2.2.2 release?

hakre7 years ago

Patch to solve remote 302 redirect injection flaw #4606

comment:4 hakre7 years ago

  • Keywords has-patch security redirect added

As the milestone-change was made from 2.2.2 to 2.2.3: no. I just tested it against 2.2.3 alpha and it's not fixed therein either.

I thought that this should be taken seriously so I analyzed it and made a fix:

Analysis

  • By effect the problem is based on unfiltered userdata.
  • Userdata is injected by the _wp_http_referer queryinfo-parameter via a get-request.
  • The reported problem can be invoked by requesting /wp-pass.php.
  • In /wp-padd.php on line 10 wordpress global redirect handler wp_redirect() is called passing wp_get_referer() as parameter.
  • wp_get_referer() injects the value into the wp_redirect() handler.
  • wp_get_referer() is defined in /wp-includes/functions.php on line 874ff.
  • It returns $_REQUEST['_wp_http_referer'] in this case which is then passed to wp_redirect().
  • wp_redirect() is defined in /wp-includes/pluggable.php on line 393ff.
  • Next to some design flaws in this routine, it does what it should: perform the redirect.
  • The problem is to pass the returned value of wp_get_referer() directly to wp_redirect() without checking for valid input.
  • A Fix has to be applied in /wp-padd.php.

Solution

  • /wp-padd.php should only redirect to adresses on the current server (domain based security).
  • This can be achieved by comparing the referer against blog-configuration.
  • This way of solving is done by the attached patch.

Testing Documentation

  1. Requested /wp-pass.php?_wp_http_referer=http://localhost/
  2. No Redirection is made.
  3. Requested /wp-pass.php?_wp_http_referer=http://webroot.loc/wordpress/
  4. Redirect is properly done.

Redirect to other Domains aren't possible any longer. The script still redirects to URLs on the same Domain.

comment:5 hakre7 years ago

correction to my report above: 2.2.3 alpha = 2.3 alpha from SVN

comment:6 foolswisdom7 years ago

  • Milestone changed from 2.2.3 to 2.3

markjaquith7 years ago

introducing wp_safe_redirect()

comment:7 markjaquith7 years ago

  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

Check out 4606.002.diff which introduces wp_safe_redirect()

wp_safe_redirect is like wp_redirect(), but it only allows server relative redirects (start with a single forward slash) or redirects that start with get_option('home'). Anything else, including URLs that start with "" or URLs on a different domain, get changed to get_option('home') . '/' before wp_redirect() is called.

This won't be used all, the time -- only when we're using a user-provided redirect.

This allows for the most backwards compatibility (as opposed to patching wp_redirect() itself.

I changed a few instances of using untrusted URLs for redirects, but there may be more.

comment:8 ryan7 years ago

+1

comment:9 markjaquith7 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6131]) Introducing wp_safe_redirect(). fixes #4606 for trunk

comment:10 markjaquith7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Issue with that first swing was that it couldn't handle relative paths, like "wp-admin/" -- which is the default redirect_to for wp-login.php

A more robust solution is coming up.

comment:11 markjaquith7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [6134]) More robust wp_safe_redirect(). Introducing wp_sanitize_redirect() for use in wp_redirect() and wp_safe_redirect(). fixes #4606

comment:12 markjaquith7 years ago

(In [6136]) Put a filter in wp_safe_redirect() so people can whitelist other domains. see #4606

comment:13 snakefoot7 years ago

  • Milestone changed from 2.3 to 2.0.12
  • Resolution fixed deleted
  • Status changed from closed to reopened

This security fix should be moved to the 2.0 branch

snakefoot7 years ago

Fix for branch 2.0

snakefoot7 years ago

Fix for branch 2.0 (With host filter)

comment:14 ryan6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [6751]) wp_safe_redirect() for 2.0. Props markjaquith and snakefoot. fixes #4606 for 2.0

Note: See TracTickets for help on using tickets.