Merge pull request #1024 from soyuka/security/json-error-normalizer

Do not expose sensitive informations (json format)

Author: dunglas

src/Hydra/Serializer/ErrorNormalizer.php

@@ -12,8 +12,8 @@
 namespace ApiPlatform\Core\Hydra\Serializer;
 
 use ApiPlatform\Core\Api\UrlGeneratorInterface;
+use ApiPlatform\Core\Problem\Serializer\ErrorNormalizerTrait;
 use Symfony\Component\Debug\Exception\FlattenException;
-use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 
 /**
@@ -26,6 +26,8 @@ final class ErrorNormalizer implements NormalizerInterface
 {
     const FORMAT = 'jsonld';
 
+    use ErrorNormalizerTrait;
+
     private $urlGenerator;
     private $debug;
 
@@ -40,16 +42,12 @@ public function __construct(UrlGeneratorInterface $urlGenerator, bool $debug = f
      */
     public function normalize($object, $format = null, array $context = [])
     {
-        $message = $object->getMessage();
         if ($this->debug) {
             $trace = $object->getTrace();
-        } elseif ($object instanceof FlattenException) {
-            $statusCode = $context['statusCode'] ?? $object->getStatusCode();
-            if ($statusCode >= 500 && $statusCode < 600) {
-                $message = Response::$statusTexts[$statusCode];
-            }
         }
 
+        $message = $this->getErrorMessage($object, $context, $this->debug);
+
         $data = [
             '@context' => $this->urlGenerator->generate('api_jsonld_context', ['shortName' => 'Error']),
             '@type' => 'hydra:Error',

src/Problem/Serializer/ErrorNormalizer.php

@@ -25,6 +25,8 @@ final class ErrorNormalizer implements NormalizerInterface
 {
     const FORMAT = 'jsonproblem';
 
+    use ErrorNormalizerTrait;
+
     private $debug;
 
     public function __construct(bool $debug = false)
@@ -37,11 +39,12 @@ public function __construct(bool $debug = false)
      */
     public function normalize($object, $format = null, array $context = [])
     {
-        $message = $object->getMessage();
         if ($this->debug) {
             $trace = $object->getTrace();
         }
 
+        $message = $this->getErrorMessage($object, $context, $this->debug);
+
         $data = [
             'type' => $context['type'] ?? 'https://tools.ietf.org/html/rfc2616#section-10',
             'title' => $context['title'] ?? 'An error occurred',

src/Problem/Serializer/ErrorNormalizerTrait.php

@@ -0,0 +1,34 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace ApiPlatform\Core\Problem\Serializer;
+
+use Symfony\Component\Debug\Exception\FlattenException;
+use Symfony\Component\HttpFoundation\Response;
+
+trait ErrorNormalizerTrait
+{
+    private function getErrorMessage($object, array $context, bool $debug = false): string
+    {
+        $message = $object->getMessage();
+
+        if ($debug) {
+            return $message;
+        } elseif ($object instanceof FlattenException) {
+            $statusCode = $context['statusCode'] ?? $object->getStatusCode();
+            if ($statusCode >= 500 && $statusCode < 600) {
+                $message = Response::$statusTexts[$statusCode];
+            }
+        }
+
+        return $message;
+    }
+}

tests/Problem/Serializer/ErrorNormalizerTest.php

@@ -13,6 +13,7 @@
 
 use ApiPlatform\Core\Problem\Serializer\ErrorNormalizer;
 use Symfony\Component\Debug\Exception\FlattenException;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * @author Kévin Dunglas <[email protected]>
@@ -53,4 +54,55 @@ public function testNormalize()
             $normalizer->normalize(new \Exception('Hello'), null, ['type' => 'https://dunglas.fr', 'title' => 'Hi'])
         );
     }
+
+    /**
+     * @dataProvider providerStatusCode
+     *
+     * @param $status            http status code of the Exception
+     * @param $originalMessage   original message of the Exception
+     * @param $debug             simulates kernel debug variable
+     */
+    public function testErrorServerNormalize($status, $originalMessage, $debug)
+    {
+        $normalizer = new ErrorNormalizer($debug);
+        $exception = FlattenException::create(new \Exception($originalMessage), $status);
+
+        $expected = [
+            'type' => 'https://tools.ietf.org/html/rfc2616#section-10',
+            'title' => 'An error occurred',
+            'detail' => ($debug || $status < 500) ? $originalMessage : Response::$statusTexts[$status],
+        ];
+
+        if ($debug) {
+            $expected['trace'] = $exception->getTrace();
+        }
+
+        $this->assertEquals($expected, $normalizer->normalize($exception, null, ['statusCode' => $status]));
+    }
+
+    public function providerStatusCode()
+    {
+        return [
+            [Response::HTTP_INTERNAL_SERVER_ERROR, 'Sensitive SQL error displayed', false],
+            [Response::HTTP_GATEWAY_TIMEOUT, 'Sensitive server error displayed', false],
+            [Response::HTTP_BAD_REQUEST, 'Bad Request Message', false],
+            [Response::HTTP_INTERNAL_SERVER_ERROR, 'Sensitive SQL error displayed', true],
+            [Response::HTTP_GATEWAY_TIMEOUT, 'Sensitive server error displayed', true],
+            [Response::HTTP_BAD_REQUEST, 'Bad Request Message', true],
+        ];
+    }
+
+    public function testErrorServerNormalizeContextStatus()
+    {
+        $normalizer = new ErrorNormalizer(false);
+        $exception = FlattenException::create(new \Exception(''), 500);
+
+        $expected = [
+            'type' => 'https://tools.ietf.org/html/rfc2616#section-10',
+            'title' => 'An error occurred',
+            'detail' => Response::$statusTexts[502],
+        ];
+
+        $this->assertEquals($expected, $normalizer->normalize($exception, null, ['statusCode' => 502]));
+    }
 }