bug #34158 [ErrorRenderer] Security fix: hide sensitive error message…

…s (dunglas)

This PR was merged into the 4.4 branch.

Discussion
----------

[ErrorRenderer] Security fix: hide sensitive error messages

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | n/a
| License       | MIT
| Doc PR        | n/a

This PR fixes a security issue. Exception messages must not be displayed except when debugging, because they can contain sensitive data including credentials.
For instance, PDO and Doctrine throw exception with message such as `The details are: SQLSTATE[HY000] [1045] Access denied for user 'root'@'db.example.com' (using password: NO)` revealing internal details about the infrastructure usful for an attacker.

Also, I still think that ErrorRenderer should be removed in favor of using the Serializer directly (see #33650 (comment)). I'll try to open some PRs to do that in tomorrow.

Commits
-------

d7d7f22 [ErrorRenderer] Security fix: hide sensitive error messages

src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginTest.php

@@ -70,6 +70,6 @@ public function testDefaultJsonLoginBadRequest()
 
         $this->assertSame(400, $response->getStatusCode());
         $this->assertSame('application/json', $response->headers->get('Content-Type'));
-        $this->assertSame(['title' => 'Bad Request', 'status' => 400, 'detail' => 'Invalid JSON.'], json_decode($response->getContent(), true));
+        $this->assertSame(['title' => 'Bad Request', 'status' => 400], json_decode($response->getContent(), true));
     }
 }

src/Symfony/Component/ErrorRenderer/ErrorRenderer/JsonErrorRenderer.php

@@ -43,9 +43,9 @@ public function render(FlattenException $exception): string
         $content = [
             'title' => $exception->getTitle(),
             'status' => $exception->getStatusCode(),
-            'detail' => $exception->getMessage(),
         ];
         if ($debug) {
+            $content['detail'] = $exception->getMessage();
             $content['exceptions'] = $exception->toArray();
         }
 

src/Symfony/Component/ErrorRenderer/ErrorRenderer/TxtErrorRenderer.php

@@ -41,9 +41,10 @@ public function render(FlattenException $exception): string
         $debug = $this->debug && ($exception->getHeaders()['X-Debug'] ?? true);
         $content = sprintf("[title] %s\n", $exception->getTitle());
         $content .= sprintf("[status] %s\n", $exception->getStatusCode());
-        $content .= sprintf("[detail] %s\n", $exception->getMessage());
 
         if ($debug) {
+            $content .= sprintf("[detail] %s\n", $exception->getMessage());
+
             foreach ($exception->toArray() as $i => $e) {
                 $content .= sprintf("[%d] %s: %s\n", $i + 1, $e['class'], $e['message']);
                 foreach ($e['trace'] as $trace) {

src/Symfony/Component/ErrorRenderer/ErrorRenderer/XmlErrorRenderer.php

@@ -42,12 +42,14 @@ public function render(FlattenException $exception): string
     {
         $debug = $this->debug && ($exception->getHeaders()['X-Debug'] ?? true);
         $title = $this->escapeXml($exception->getTitle());
-        $message = $this->escapeXml($exception->getMessage());
         $statusCode = $this->escapeXml($exception->getStatusCode());
         $charset = $this->escapeXml($this->charset);
 
         $exceptions = '';
+        $message = '';
         if ($debug) {
+            $message = '<detail>'.$this->escapeXml($exception->getMessage()).'</detail>';
+
             $exceptions .= '<exceptions>';
             foreach ($exception->toArray() as $e) {
                 $exceptions .= sprintf('<exception class="%s" message="%s"><traces>', $e['class'], $this->escapeXml($e['message']));
@@ -71,7 +73,7 @@ public function render(FlattenException $exception): string
 <problem xmlns="urn:ietf:rfc:7807">
     <title>{$title}</title>
     <status>{$statusCode}</status>
-    <detail>{$message}</detail>
+    {$message}
     {$exceptions}
 </problem>
 EOF;

src/Symfony/Component/ErrorRenderer/Tests/Command/DebugCommandTest.php

@@ -56,8 +56,7 @@ public function testFormatArgument()
         $this->assertSame(<<<TXT
 {
     "title": "Internal Server Error",
-    "status": 500,
-    "detail": "This is a sample exception."
+    "status": 500
 }
 
 TXT

src/Symfony/Component/ErrorRenderer/Tests/ErrorRenderer/JsonErrorRendererTest.php

@@ -44,8 +44,7 @@ public function getRenderData(): iterable
         $expectedNonDebug = <<<JSON
 {
     "title": "Internal Server Error",
-    "status": 500,
-    "detail": "Foo"
+    "status": 500
 }
 JSON;
 

src/Symfony/Component/ErrorRenderer/Tests/ErrorRenderer/TxtErrorRendererTest.php

@@ -39,7 +39,6 @@ public function getRenderData(): iterable
         $expectedNonDebug = <<<TXT
 [title] Internal Server Error
 [status] 500
-[detail] Foo
 TXT;
 
         yield '->render() returns the TXT content WITH stack traces in debug mode' => [

src/Symfony/Component/ErrorRenderer/Tests/ErrorRenderer/XmlErrorRendererTest.php

@@ -43,7 +43,7 @@ public function getRenderData(): iterable
 <problem xmlns="urn:ietf:rfc:7807">
     <title>Internal Server Error</title>
     <status>500</status>
-    <detail>Foo</detail>
+    
     
 </problem>
 XML;

src/Symfony/Component/HttpKernel/Tests/Controller/ErrorControllerTest.php

@@ -61,7 +61,7 @@ public function getInvokeControllerDataProvider()
             $request,
             FlattenException::createFromThrowable(new \Exception('foo')),
             500,
-            '{"title": "Internal Server Error","status": 500,"detail": "foo"}',
+            '{"title": "Internal Server Error","status": 500}',
         ];
 
         $request = new Request();
@@ -70,7 +70,7 @@ public function getInvokeControllerDataProvider()
             $request,
             FlattenException::createFromThrowable(new HttpException(405, 'Invalid request.')),
             405,
-            '{"title": "Method Not Allowed","status": 405,"detail": "Invalid request."}',
+            '{"title": "Method Not Allowed","status": 405}',
         ];
 
         $request = new Request();
@@ -79,7 +79,7 @@ public function getInvokeControllerDataProvider()
             $request,
             FlattenException::createFromThrowable(new HttpException(405, 'Invalid request.')),
             405,
-            '{"title": "Method Not Allowed","status": 405,"detail": "Invalid request."}',
+            '{"title": "Method Not Allowed","status": 405}',
         ];
 
         $request = new Request();