Merge pull request #10584 from creative-commoners/pulls/4.11/cve-2022…

…-38462 Don't allow CRLF in header values
silverstripe · Nov 21, 2022 · e5b8110 · e5b8110
2 parents b17b29e + d3c2857
commit e5b8110
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 1 deletion.
diff --git a/src/Control/HTTPResponse.php b/src/Control/HTTPResponse.php
@@ -267,7 +267,7 @@ public function getBody()
     public function addHeader($header, $value)
     {
         $header = strtolower($header ?? '');
-        $this->headers[$header] = $value;
+        $this->headers[$header] = $this->sanitiseHeader($value);
         return $this;
     }
 
@@ -310,6 +310,14 @@ public function removeHeader($header)
         return $this;
     }
 
+    /**
+     * Sanitise header values to avoid possible XSS vectors
+     */
+    private function sanitiseHeader(string $value): string
+    {
+        return preg_replace('/\v/', '', $value);
+    }
+
     /**
      * @param string $dest
      * @param int $code

diff --git a/tests/php/Control/HTTPResponseTest.php b/tests/php/Control/HTTPResponseTest.php
@@ -45,6 +45,26 @@ public function testRemoveHeader()
         $this->assertEmpty($response->getHeader('X-Animal'));
     }
 
+    public function providerSanitiseHeaders()
+    {
+        return [
+            'plain text is retained' => ['some arbitrary value1', 'some arbitrary value1'],
+            'special chars are retained' => ['`~!@#$%^&*()_+-=,./<>?;\':"[]{}\\|', '`~!@#$%^&*()_+-=,./<>?;\':"[]{}\\|'],
+            'line breaks are removed' => ['no line breaks', "n\ro line \nbreaks\r\n"],
+        ];
+    }
+
+    /**
+     * @dataProvider providerSanitiseHeaders
+     */
+    public function testSanitiseHeaders(string $expected, string $value)
+    {
+        $response = new HTTPResponse();
+
+        $response->addHeader('X-Sanitised', $value);
+        $this->assertSame($expected, $response->getHeader('X-Sanitised'));
+    }
+
     public function providerTestValidStatusCodes()
     {
         return [