From 49ffef2579a7814c75415b31fbce4421849169f8 Mon Sep 17 00:00:00 2001
From: Helmut Hummel <helmut.hummel@typo3.org>
Date: Sun, 22 Nov 2015 13:09:58 +0100
Subject: [PATCH] [TASK] Disallow multi-line HTTP headers
PHP removed the support for this deprecated HTTP specification
in recent versions of PHP, thus we should remove these as well.
Besides that, we add an additional check for newlines
in GeneralUtility::locationHeaderUrl() to prevent potential
issues with Internet Explorer.
These lines can be removed once the minimum PHP requirement
are raised.
Releases: master, 6.2
Resolves: #58816
Change-Id: I38d26affd31913b82a972ac90ebf906a45b92e05
Reviewed-on: https://review.typo3.org/44898
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
---
typo3/sysext/core/Classes/Http/Message.php | 7 ++-----
.../core/Classes/Utility/GeneralUtility.php | 5 +++++
.../core/Tests/Unit/Http/MessageTest.php | 19 ++-----------------
3 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/typo3/sysext/core/Classes/Http/Message.php b/typo3/sysext/core/Classes/Http/Message.php
index 1197228cb3bb..c28e2abdb16f 100644
--- a/typo3/sysext/core/Classes/Http/Message.php
+++ b/typo3/sysext/core/Classes/Http/Message.php
@@ -460,11 +460,8 @@ class Message implements MessageInterface
{
$value = (string)$value;
- // Look for:
- // \n not preceded by \r, OR
- // \r not followed by \n, OR
- // \r\n not followed by space or horizontal tab; these are all CRLF attacks
- if (preg_match("#(?:(?:(?<!\r)\n)|(?:\r(?!\n))|(?:\r\n(?![ \t])))#", $value)) {
+ // Any occurence of \r or \n is invalid
+ if (strpbrk($value, "\r\n") !== false) {
return false;
}
diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
index 65ebca4ab98d..b1b8b64df848 100755
--- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php
+++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
@@ -3146,6 +3146,7 @@ Connection: close
*
* @param string $path URL / path to prepend full URL addressing to.
* @return string
+ * @throws \InvalidArgumentException
*/
public static function locationHeaderUrl($path)
{
@@ -3157,6 +3158,10 @@ Connection: close
// No scheme either
$path = self::getIndpEnv('TYPO3_REQUEST_DIR') . $path;
}
+ // Can be removed once minimum PHP requirement is at least 5.5.22 or 5.6.6
+ if (strpbrk($path, "\r\n") !== false) {
+ throw new \InvalidArgumentException('HTTP header injection attempt in "' . $path . '"', 1448194036);
+ }
return $path;
}
diff --git a/typo3/sysext/core/Tests/Unit/Http/MessageTest.php b/typo3/sysext/core/Tests/Unit/Http/MessageTest.php
index 4d0f2552d643..a0562484aa70 100644
--- a/typo3/sysext/core/Tests/Unit/Http/MessageTest.php
+++ b/typo3/sysext/core/Tests/Unit/Http/MessageTest.php
@@ -285,6 +285,8 @@ class MessageTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
'array-value-with-lf' => ['X-Foo-Bar', ["value\ninjection"]],
'array-value-with-crlf' => ['X-Foo-Bar', ["value\r\ninjection"]],
'array-value-with-2crlf' => ['X-Foo-Bar', ["value\r\n\r\ninjection"]],
+ 'multi-line-header-space' => ['X-Foo-Bar', "value\r\n injection"],
+ 'multi-line-header-tab' => ['X-Foo-Bar', "value\r\n\tinjection"],
];
}
@@ -308,21 +310,4 @@ class MessageTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
$this->message->withAddedHeader($name, $value);
}
- /**
- * @test
- */
- public function testWithHeaderAllowsHeaderContinuations()
- {
- $message = $this->message->withHeader('X-Foo-Bar', "value,\r\n second value");
- $this->assertEquals("value,\r\n second value", $message->getHeaderLine('X-Foo-Bar'));
- }
-
- /**
- * @test
- */
- public function testWithAddedHeaderAllowsHeaderContinuations()
- {
- $message = $this->message->withAddedHeader('X-Foo-Bar', "value,\r\n second value");
- $this->assertEquals("value,\r\n second value", $message->getHeaderLine('X-Foo-Bar'));
- }
}
--
GitLab