[TASK] Disallow multi-line HTTP headers 98/44898/3
authorHelmut Hummel <helmut.hummel@typo3.org>
Sun, 22 Nov 2015 12:09:58 +0000 (13:09 +0100)
committerWouter Wolters <typo3@wouterwolters.nl>
Sun, 22 Nov 2015 12:58:03 +0000 (13:58 +0100)
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
typo3/sysext/core/Classes/Utility/GeneralUtility.php
typo3/sysext/core/Tests/Unit/Http/MessageTest.php

index 1197228..c28e2ab 100644 (file)
@@ -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;
         }
 
index 65ebca4..b1b8b64 100755 (executable)
@@ -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;
     }
 
index 4d0f255..a056248 100644 (file)
@@ -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'));
-    }
 }