From 048979993e94f74946fc7ed6b7ac612db2c0a01e Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 6 Dec 2012 13:50:59 +0100 Subject: [PATCH 1/2] [HttpFoundation] added a check for the host header value --- .../Component/HttpFoundation/Request.php | 24 ++++++++++++------- .../Component/HttpFoundation/RequestTest.php | 14 ++++++++--- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index 03c7e03a5f..f9c9673b8c 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -696,6 +696,8 @@ class Request * * @return string * + * @throws \UnexpectedValueException when the host name is invalid + * * @api */ public function getHost() @@ -703,19 +705,23 @@ class Request if (self::$trustProxy && $host = $this->headers->get('X_FORWARDED_HOST')) { $elements = explode(',', $host); - $host = trim($elements[count($elements) - 1]); - } else { - if (!$host = $this->headers->get('HOST')) { - if (!$host = $this->server->get('SERVER_NAME')) { - $host = $this->server->get('SERVER_ADDR', ''); - } + $host = $elements[count($elements) - 1]; + } elseif (!$host = $this->headers->get('HOST')) { + if (!$host = $this->server->get('SERVER_NAME')) { + $host = $this->server->get('SERVER_ADDR', ''); } } - // Remove port number from host - $host = preg_replace('/:\d+$/', '', $host); + // Trim and remove port number from host + $host = trim(preg_replace('/:\d+$/', '', $host)); - return trim($host); + // as the host can come from the user (HTTP_HOST and depending on the configuration, SERVER_NAME too can come from the user) + // check that it does not contain forbidden characters (see RFC 952 and RFC 2181) + if ($host && !preg_match('/^\[?(?:[a-zA-Z0-9-:\]_]+\.?)+$/', $host)) { + throw new \UnexpectedValueException('Invalid Host'); + } + + return $host; } /** diff --git a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php index 43e083aadd..d006a4a811 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php @@ -417,9 +417,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase $this->assertEquals('foo=1&foo=2', $request->getQueryString(), '->getQueryString() allows repeated parameters'); } - /** - * @covers Symfony\Component\HttpFoundation\Request::getHost - */ public function testGetHost() { $request = new Request(); @@ -458,6 +455,17 @@ class RequestTest extends \PHPUnit_Framework_TestCase $request->initialize(array(), array(), array(), array(), array(), array('SERVER_NAME' => 'www.exemple.com', 'HTTP_HOST' => 'www.host.com')); $this->assertEquals('www.host.com', $request->getHost(), '->getHost() value from Host header has priority over SERVER_NAME '); + + } + + /** + * @expectedException RuntimeException + */ + public function testGetHostWithFakeHttpHostValue() + { + $request = new Request(); + $request->initialize(array(), array(), array(), array(), array(), array('HTTP_HOST' => 'www.host.com?query=string')); + $request->getHost(); } /** From 447ff915df4dc896bb5e2f7dc069c9516ffe92d6 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 7 Dec 2012 10:25:02 +0100 Subject: [PATCH 2/2] [HttpFoundation] changed UploadedFile::move() to use move_uploaded_file() when possible (closes #5878, closes #6185) --- .../Component/HttpFoundation/File/File.php | 23 ++++++++++++------- .../HttpFoundation/File/UploadedFile.php | 17 ++++++++++++-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/File/File.php b/src/Symfony/Component/HttpFoundation/File/File.php index 43b8c0d265..de4fb9d17b 100644 --- a/src/Symfony/Component/HttpFoundation/File/File.php +++ b/src/Symfony/Component/HttpFoundation/File/File.php @@ -523,6 +523,20 @@ class File extends \SplFileInfo * @api */ public function move($directory, $name = null) + { + $target = $this->getTargetFile($directory, $name); + + if (!@rename($this->getPathname(), $target)) { + $error = error_get_last(); + throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); + } + + @chmod($target, 0666 & ~umask()); + + return $target; + } + + protected function getTargetFile($directory, $name = null) { if (!is_dir($directory)) { if (false === @mkdir($directory, 0777, true)) { @@ -534,14 +548,7 @@ class File extends \SplFileInfo $target = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getBasename() : $this->getName($name)); - if (!@rename($this->getPathname(), $target)) { - $error = error_get_last(); - throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); - } - - chmod($target, 0666); - - return new File($target); + return new File($target, false); } /** diff --git a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php index 7b337da9ff..0830a8a06c 100644 --- a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php +++ b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php @@ -189,8 +189,21 @@ class UploadedFile extends File */ public function move($directory, $name = null) { - if ($this->isValid() && ($this->test || is_uploaded_file($this->getPathname()))) { - return parent::move($directory, $name); + if ($this->isValid()) { + if ($this->test) { + return parent::move($directory, $name); + } elseif (is_uploaded_file($this->getPathname())) { + $target = $this->getTargetFile($directory, $name); + + if (!@move_uploaded_file($this->getPathname(), $target)) { + $error = error_get_last(); + throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); + } + + @chmod($target, 0666 & ~umask()); + + return $target; + } } throw new FileException(sprintf('The file "%s" has not been uploaded via Http', $this->getPathname()));