revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 14:00:56 +01:00
Code Issues Releases Wiki Activity

Defuse a "Host:" header attack

Browse source 
Summary:
Django released a security update recently dealing with malicious "Host" headers:

https://www.djangoproject.com/weblog/2012/oct/17/security/

We're vulnerable to the same attack. Plug the hole.

The risk here is that an attacker does something like this:

  # Register "evil.com".
  # Point it at secure.phabricator.com in DNS.
  # Send a legitimate user a link to "secure.phabricator.com:ignored@evil.com".
  # They login and get cookies. Normally Phabricator refuses to set cookies on domains it does not recognize.
  # The attacker now points "evil.com" at his own servers and reads the auth cookies on the next request.

Test Plan: Unit tests.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3766
This commit is contained in:
epriestley 2012-10-22 10:49:06 -07:00
parent 96b5d0e74a
commit 38c83ef846
2 changed files with 26 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/aphront/AphrontRequest.php
View file

@ -65,10 +65,14 @@ final class AphrontRequest {
}
final public function getHost() {
// The "Host" header may include a port number; if so, ignore it. We can't
// use PhutilURI since there's no URI scheme.
list($actual_host) = explode(':', $this->host, 2);
return $actual_host;
// The "Host" header may include a port number, or may be a malicious
// header in the form "realdomain.com:ignored@evil.com". Invoke the full
// parser to extract the real domain correctly. See here for coverage of
// a similar issue in Django:
//
// https://www.djangoproject.com/weblog/2012/oct/17/security/
$uri = new PhutilURI('http://'.$this->host);
return $uri->getDomain();
}

19
src/aphront/__tests__/AphrontRequestTestCase.php
View file

@ -19,7 +19,7 @@
final class AphrontRequestTestCase extends PhabricatorTestCase {
public function testRequestDataAccess() {
$r = new AphrontRequest('http://example.com/', '/');
$r = new AphrontRequest('example.com', '/');
$r->setRequestData(
array(
'str_empty' => '',
@ -78,4 +78,21 @@ final class AphrontRequestTestCase extends PhabricatorTestCase {
$this->assertEqual(false, $r->getExists('does-not-exist'));
}
public function testHostAttacks() {
static $tests = array(
'domain.com' => 'domain.com',
'domain.com:80' => 'domain.com',
'evil.com:evil.com@real.com' => 'real.com',
'evil.com:evil.com@real.com:80' => 'real.com',
);
foreach ($tests as $input => $expect) {
$r = new AphrontRequest($input, '/');
$this->assertEqual(
$expect,
$r->getHost(),
'Host: '.$input);
}
}
}