Skip to content

Commit

Permalink
Merge remote-tracking branch 'phpbb-security/ticket/security-180' int…
Browse files Browse the repository at this point in the history
…o prep-release-3.0.14

* phpbb-security/ticket/security-180:
  [ticket/security-180] Add tests for redirecting to main URL
  [ticket/security-180] Always fail when redirecting to an insecure URL
  [ticket/security-180] Make sure that redirect goes to full URL plus slash
  [ticket/security-180] Check if redirect URL contains board URL
  • Loading branch information
bantu committed Apr 28, 2015
2 parents d833f29 + 18fc621 commit 1a33506
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
8 changes: 7 additions & 1 deletion phpBB/includes/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -2492,7 +2492,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
// Attention: only able to redirect within the same domain if $disable_cd_check is false (yourdomain.com -> www.yourdomain.com will not work)
if (!$disable_cd_check && $url_parts['host'] !== $user->host)
{
$url = generate_board_url();
trigger_error('Tried to redirect to potentially insecure url.', E_USER_ERROR);
}
}
else if ($url[0] == '/')
Expand Down Expand Up @@ -2579,6 +2579,12 @@ function redirect($url, $return = false, $disable_cd_check = false)
}
}

// Make sure we don't redirect to external URLs
if (!$disable_cd_check && strpos($url, generate_board_url(true) . '/') !== 0)
{
trigger_error('Tried to redirect to potentially insecure url.', E_USER_ERROR);
}

// Make sure no linebreaks are there... to prevent http response splitting for PHP < 4.4.2
if (strpos(urldecode($url), "\n") !== false || strpos(urldecode($url), "\r") !== false || strpos($url, ';') !== false)
{
Expand Down
9 changes: 7 additions & 2 deletions tests/security/redirect_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@ static public function provider()
{
// array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false))
return array(
array('data://x', false, 'http://localhost/phpBB'),
array('data://x', 'Tried to redirect to potentially insecure url.', false),
array('bad://localhost/phpBB/index.php', 'Tried to redirect to potentially insecure url.', false),
array('http://www.otherdomain.com/somescript.php', false, 'http://localhost/phpBB'),
array('http://www.otherdomain.com/somescript.php', 'Tried to redirect to potentially insecure url.', false),
array("http://localhost/phpBB/memberlist.php\n\rConnection: close", 'Tried to redirect to potentially insecure url.', false),
array('javascript:test', false, 'http://localhost/phpBB/../javascript:test'),
array('http://localhost/phpBB/index.php;url=', 'Tried to redirect to potentially insecure url.', false),
array('https://foobar.com\@http://localhost/phpBB', 'Tried to redirect to potentially insecure url.', false),
array('https://foobar.com\@localhost/troll/http://localhost/', 'Tried to redirect to potentially insecure url.', false),
array('http://localhost.foobar.com\@localhost/troll/http://localhost/', 'Tried to redirect to potentially insecure url.', false),
array('http://localhost/phpBB', false, 'http://localhost/phpBB'),
array('http://localhost/phpBB/', false, 'http://localhost/phpBB/'),
);
}

Expand Down

0 comments on commit 1a33506

Please sign in to comment.