Make WordPress Core

Opened 17 years ago

Closed 17 years ago

#4606 closed defect (bug) (fixed)

Redirection Vulnerability in wp-pass.php

Reported by: snakefoot's profile snakefoot Owned by: markjaquith's profile 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 17 years ago.
Patch to solve remote 302 redirect injection flaw #4606
4606.002.diff (1.8 KB) - added by markjaquith 17 years ago.
introducing wp_safe_redirect()
wp2-4606.002.diff (1.9 KB) - added by snakefoot 17 years ago.
Fix for branch 2.0
wp2-4606.003.diff (3.2 KB) - added by snakefoot 17 years ago.
Fix for branch 2.0 (With host filter)

Download all attachments as: .zip

Change History (18)

#1 follow-up: @matt
17 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.

#2 @foolswisdom
17 years ago

  • Milestone changed from 2.2.2 to 2.2.3

#3 in reply to: ↑ 1 @Nazgul
17 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?

@hakre
17 years ago

Patch to solve remote 302 redirect injection flaw #4606

#4 @hakre
17 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.

#5 @hakre
17 years ago

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

#6 @foolswisdom
17 years ago

  • Milestone changed from 2.2.3 to 2.3

@markjaquith
17 years ago

introducing wp_safe_redirect()

#7 @markjaquith
17 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.

#9 @markjaquith
17 years ago

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

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

#10 @markjaquith
17 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.

#11 @markjaquith
17 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

#12 @markjaquith
17 years ago

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

#13 @snakefoot
17 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

@snakefoot
17 years ago

Fix for branch 2.0

@snakefoot
17 years ago

Fix for branch 2.0 (With host filter)

#14 @ryan
17 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.