Merge pull request #183 from michelsalib/xml-document-whitelist

Support whitelist for xml document types

DependencyInjection/Configuration.php

@@ -28,6 +28,9 @@ class Configuration implements ConfigurationInterface
     private $debug;
     private $factories;
 
+    /**
+     * @param boolean $debug
+     */
     public function __construct($debug = false, array $factories = array())
     {
         $this->debug = $debug;
@@ -158,6 +161,15 @@ class Configuration implements ConfigurationInterface
                             ->end()
                         ->end()
                     ->end()
+                    ->arrayNode('xml')
+                        ->fixXmlConfig('whitelisted-doctype', 'doctype_whitelist')
+                        ->addDefaultsIfNotSet()
+                        ->children()
+                            ->arrayNode('doctype_whitelist')
+                                ->prototype('scalar')->end()
+                            ->end()
+                        ->end()
+                    ->end()
                 ->end()
             ->end()
         ;

DependencyInjection/JMSSerializerExtension.php

@@ -19,16 +19,12 @@
 namespace JMS\SerializerBundle\DependencyInjection;
 
 use Symfony\Component\HttpKernel\DependencyInjection\ConfigurableExtension;
-use Symfony\Component\HttpKernel\KernelInterface;
 use Symfony\Component\DependencyInjection\Alias;
-use Symfony\Component\DependencyInjection\DefinitionDecorator;
 use JMS\SerializerBundle\Exception\RuntimeException;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\Config\Definition\Processor;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 
 class JMSSerializerExtension extends ConfigurableExtension
 {
@@ -95,7 +91,7 @@ class JMSSerializerExtension extends ConfigurableExtension
         // metadata
         if ('none' === $config['metadata']['cache']) {
             $container->removeAlias('jms_serializer.metadata.cache');
-        } else if ('file' === $config['metadata']['cache']) {
+        } elseif ('file' === $config['metadata']['cache']) {
             $container
                 ->getDefinition('jms_serializer.metadata.cache.file_cache')
                 ->replaceArgument(0, $config['metadata']['file_cache']['dir'])
@@ -149,6 +145,10 @@ class JMSSerializerExtension extends ConfigurableExtension
         $container
             ->setParameter('jms_serializer.json_serialization_visitor.options', $config['visitors']['json']['options'])
         ;
+
+        $container
+            ->setParameter('jms_serializer.xml_deserialization_visitor.doctype_whitelist', $config['visitors']['xml']['doctype_whitelist'])
+        ;
     }
 
     public function getConfiguration(array $config, ContainerBuilder $container)

Resources/config/services.xml

@@ -33,6 +33,7 @@
         <parameter key="jms_serializer.json_deserialization_visitor.class">JMS\SerializerBundle\Serializer\JsonDeserializationVisitor</parameter>
         <parameter key="jms_serializer.xml_serialization_visitor.class">JMS\SerializerBundle\Serializer\XmlSerializationVisitor</parameter>
         <parameter key="jms_serializer.xml_deserialization_visitor.class">JMS\SerializerBundle\Serializer\XmlDeserializationVisitor</parameter>
+        <parameter key="jms_serializer.xml_deserialization_visitor.doctype_whitelist" type="collection"></parameter>
         <parameter key="jms_serializer.yaml_serialization_visitor.class">JMS\SerializerBundle\Serializer\YamlSerializationVisitor</parameter>
 
         <parameter key="jms_serializer.object_based_custom_handler.class">JMS\SerializerBundle\Serializer\Handler\ObjectBasedCustomHandler</parameter>
@@ -148,6 +149,9 @@
             <argument type="service" id="jms_serializer.naming_strategy" />
             <argument type="collection" /><!-- Custom Handlers -->
             <argument type="service" id="jms_serializer.object_constructor" />
+            <call method="setDoctypeWhitelist">
+                <argument>%jms_serializer.xml_deserialization_visitor.doctype_whitelist%</argument>
+            </call>
             <tag name="jms_serializer.deserialization_visitor" format="xml" />
         </service>
         <service id="jms_serializer.yaml_serialization_visitor" class="%jms_serializer.yaml_serialization_visitor.class%" public="false">

Resources/doc/configuration.rst

@@ -14,7 +14,7 @@ values:
 .. configuration-block ::
 
     .. code-block :: yaml
-    
+
         # config.yml
         jms_serializer:
             handlers:
@@ -25,17 +25,17 @@ values:
                 array_collection: true
                 form_error: true
                 constraint_violation: true
-    
+
             property_naming:
                 separator:  _
                 lower_case: true
-    
+
             metadata:
                 cache: file
                 debug: "%kernel.debug%"
                 file_cache:
                     dir: "%kernel.cache_dir%/serializer"
-    
+
                 # Using auto-detection, the mapping files for each bundle will be
                 # expected in the Resources/config/serializer directory.
                 #
@@ -43,7 +43,7 @@ values:
                 # class: My\FooBundle\Entity\User
                 # expected path: @MyFooBundle/Resources/config/serializer/Entity.User.(yml|xml|php)
                 auto_detection: true
-    
+
                 # if you don't want to use auto-detection, you can also define the
                 # namespace prefix and the corresponding directory explicitly
                 directories:
@@ -52,36 +52,41 @@ values:
                         path: "@MyFooBundle/Resources/config/serializer"
                     another-name:
                         namespace_prefix: "My\\BarBundle"
-                        path: "@MyBarBundle/Resources/config/serializer"    
+                        path: "@MyBarBundle/Resources/config/serializer"
+
+            visitors:
+                xml:
+                    doctype_whitelist:
+                        - '<!DOCTYPE authorized SYSTEM "http://some_url">' # an authorized document type for xml deserialization
 
     .. code-block :: xml
-    
+
         <!-- config.xml -->
         <jms-serializer>
             <handlers>
                 <object-based />
-                <datetime 
+                <datetime
                     format="Y-mdTH:i:s"
                     default-timezone="UTC" />
                 <array-collection />
                 <form-error />
-                <constraint-violation /> 
+                <constraint-violation />
             </handlers>
-            
+
             <property-naming
                 seperator="_"
                 lower-case="true" />
-                
+
             <metadata
                 cache="file"
                 debug="%kernel.debug%"
                 auto-detection="true">
-                
+
                 <file-cache dir="%kernel.cache_dir%/serializer" />
-                
+
                 <!-- If auto-detection is enabled, mapping files for each bundle will
-                     be expected in the Resources/config/serializer directory. 
-                     
+                     be expected in the Resources/config/serializer directory.
+
                      Example:
                      class: My\FooBundle\Entity\User
                      expected path: @MyFooBundle/Resources/config/serializer/Entity.User.(yml|xml|php)
@@ -90,5 +95,11 @@ values:
                     namespace-prefix="My\FooBundle"
                     path="@MyFooBundle/Resources/config/serializer" />
             </metadata>
+
+            <visitors>
+                <xml>
+                    <whitelisted-doctype><![CDATA[<!DOCTYPE...>]]></whitelisted-doctype>
+                    <whitelisted-doctype><![CDATA[<!DOCTYPE...>]]></whitelisted-doctype>
+                </xml>
+            </visitors>
         </jms-serializer>
-    

Serializer/XmlDeserializationVisitor.php

@@ -36,6 +36,7 @@ class XmlDeserializationVisitor extends AbstractDeserializationVisitor
     private $result;
     private $navigator;
     private $disableExternalEntities;
+    private $doctypeWhitelist = array();
 
     public function __construct(PropertyNamingStrategyInterface $namingStrategy, array $customHandlers, ObjectConstructorInterface $objectConstructor, $disableExternalEntities = true)
     {
@@ -67,7 +68,13 @@ class XmlDeserializationVisitor extends AbstractDeserializationVisitor
         $dom->loadXML($data);
         foreach ($dom->childNodes as $child) {
             if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) {
-                throw new \InvalidArgumentException('Document types are not allowed.');
+                $internalSubset = str_replace(PHP_EOL, '', $child->internalSubset);
+                if (!in_array($internalSubset, $this->doctypeWhitelist, true)) {
+                    throw new \InvalidArgumentException(sprintf(
+                        'The document type "%s" is not allowed. If it is safe, you may add it to the whitelist configuration.',
+                        $internalSubset
+                    ));
+                }
             }
         }
 
@@ -99,7 +106,7 @@ class XmlDeserializationVisitor extends AbstractDeserializationVisitor
 
         if ('true' === $data) {
             $data = true;
-        } else if ('false' === $data) {
+        } elseif ('false' === $data) {
             $data = false;
         } else {
             throw new RuntimeException(sprintf('Could not convert data to boolean. Expected "true", or "false", but got %s.', json_encode($data)));
@@ -207,7 +214,7 @@ class XmlDeserializationVisitor extends AbstractDeserializationVisitor
         $name = $this->namingStrategy->translateName($metadata);
 
         if (!$metadata->type) {
-            throw new RuntimeException(sprintf('You must define a type for %s::$%s.', $metadata->reflection->getDeclaringClass()->getName(), $metadata->name));
+            throw new RuntimeException(sprintf('You must define a type for %s::$%s.', $metadata->reflection->class, $metadata->name));
         }
 
         if ($metadata->xmlAttribute) {
@@ -305,4 +312,20 @@ class XmlDeserializationVisitor extends AbstractDeserializationVisitor
     {
         return $this->result;
     }
-}
+
+    /**
+     * @param array<string> $doctypeWhitelist
+     */
+    public function setDoctypeWhitelist(array $doctypeWhitelist)
+    {
+        $this->doctypeWhitelist = $doctypeWhitelist;
+    }
+
+    /**
+     * @return array<string>
+     */
+    public function getDoctypeWhitelist()
+    {
+        return $this->doctypeWhitelist;
+    }
+}

Tests/DependencyInjection/JMSSerializerExtensionTest.php

@@ -120,6 +120,32 @@ class JMSSerializerExtensionTest extends \PHPUnit_Framework_TestCase
         return $configs;
     }
 
+    /**
+     * @dataProvider getXmlVisitorWhitelists
+     */
+    public function testXmlVisitorOptions($expectedOptions, $config)
+    {
+        $container = $this->getContainerForConfig(array($config));
+        $this->assertSame($expectedOptions, $container->get('jms_serializer.xml_deserialization_visitor')->getDoctypeWhitelist());
+    }
+
+    public function getXmlVisitorWhitelists()
+    {
+        $configs = array();
+
+        $configs[] = array(array('good document', 'other good document'), array(
+            'visitors' => array(
+                'xml' => array(
+                    'doctype_whitelist' => array('good document', 'other good document'),
+                )
+            )
+        ));
+
+        $configs[] = array(array(), array());
+
+        return $configs;
+    }
+
     private function getContainerForConfig(array $configs, KernelInterface $kernel = null)
     {
         if (null === $kernel) {
@@ -154,4 +180,4 @@ class JMSSerializerExtensionTest extends \PHPUnit_Framework_TestCase
 
         return $container;
     }
-}
+}

Tests/Serializer/XmlSerializationTest.php

@@ -18,10 +18,16 @@
 
 namespace JMS\SerializerBundle\Tests\Serializer;
 
+use Metadata\MetadataFactory;
+use Doctrine\Common\Annotations\AnnotationReader;
 use JMS\SerializerBundle\Tests\Fixtures\InvalidUsageOfXmlValue;
+use JMS\SerializerBundle\Serializer\Construction\UnserializeObjectConstructor;
+use JMS\SerializerBundle\Serializer\Naming\CamelCaseNamingStrategy;
+use JMS\SerializerBundle\Serializer\Naming\SerializedNameAnnotationStrategy;
+use JMS\SerializerBundle\Serializer\XmlDeserializationVisitor;
+use JMS\SerializerBundle\Metadata\Driver\AnnotationDriver;
+use JMS\SerializerBundle\Serializer\Serializer;
 use JMS\SerializerBundle\Exception\InvalidArgumentException;
-use JMS\SerializerBundle\Annotation\Type;
-use JMS\SerializerBundle\Annotation\XmlValue;
 use JMS\SerializerBundle\Tests\Fixtures\PersonCollection;
 use JMS\SerializerBundle\Tests\Fixtures\PersonLocation;
 use JMS\SerializerBundle\Tests\Fixtures\Person;
@@ -65,7 +71,7 @@ class XmlSerializationTest extends BaseSerializationTest
 
     /**
      * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Document types are not allowed
+     * @expectedExceptionMessage The document type "<!DOCTYPE author [<!ENTITY foo SYSTEM "php://filter/read=convert.base64-encode/resource=XmlSerializationTest.php">]>" is not allowed. If it is safe, you may add it to the whitelist configuration.
      */
     public function testExternalEntitiesAreDisabledByDefault()
     {
@@ -80,32 +86,60 @@ class XmlSerializationTest extends BaseSerializationTest
 
     /**
      * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Document types are not allowed
+     * @expectedExceptionMessage The document type "<!DOCTYPE foo>" is not allowed. If it is safe, you may add it to the whitelist configuration.
      */
     public function testDocumentTypesAreNotAllowed()
     {
         $this->deserialize('<?xml version="1.0"?><!DOCTYPE foo><foo></foo>', 'stdClass');
     }
 
-    public function testVirtualAttributes() {
+    public function testWhitelistedDocumentTypesAreAllowed()
+    {
+        $xmlVisitor = new XmlDeserializationVisitor(
+            new SerializedNameAnnotationStrategy(new CamelCaseNamingStrategy()),
+            $this->getDeserializationHandlers(),
+            new UnserializeObjectConstructor()
+        );
+        $xmlVisitor->setDoctypeWhitelist(array(
+            '<!DOCTYPE authorized SYSTEM "http://authorized_url.dtd">',
+            '<!DOCTYPE author [<!ENTITY foo SYSTEM "php://filter/read=convert.base64-encode/resource='.basename(__FILE__).'">]>'));
+
+        $serializer = new Serializer(new MetadataFactory(new AnnotationDriver(new AnnotationReader())), array(), array('xml' => $xmlVisitor));
+
+        $serializer->deserialize('<?xml version="1.0"?>
+            <!DOCTYPE authorized SYSTEM "http://authorized_url.dtd">
+            <foo></foo>', 'stdClass', 'xml');
+
+        $serializer->deserialize('<?xml version="1.0"?>
+            <!DOCTYPE author [
+                <!ENTITY foo SYSTEM "php://filter/read=convert.base64-encode/resource='.basename(__FILE__).'">
+            ]>
+            <foo></foo>', 'stdClass', 'xml');
+    }
+
+    public function testVirtualAttributes()
+    {
         $serializer = $this->getSerializer();
         $serializer->setGroups(array('attributes'));
         $this->assertEquals($this->getContent('virtual_attributes'), $serializer->serialize(new ObjectWithVirtualXmlProperties(),'xml'));
     }
 
-    public function testVirtualValues() {
+    public function testVirtualValues()
+    {
         $serializer = $this->getSerializer();
         $serializer->setGroups(array('values'));
         $this->assertEquals($this->getContent('virtual_values'), $serializer->serialize(new ObjectWithVirtualXmlProperties(),'xml'));
     }
 
-    public function testVirtualXmlList() {
+    public function testVirtualXmlList()
+    {
         $serializer = $this->getSerializer();
         $serializer->setGroups(array('list'));
         $this->assertEquals($this->getContent('virtual_properties_list'), $serializer->serialize(new ObjectWithVirtualXmlProperties(),'xml'));
     }
 
-    public function testVirtualXmlMap() {
+    public function testVirtualXmlMap()
+    {
         $serializer = $this->getSerializer();
         $serializer->setGroups(array('map'));
         $this->assertEquals($this->getContent('virtual_properties_map'), $serializer->serialize(new ObjectWithVirtualXmlProperties(),'xml'));
@@ -117,6 +151,9 @@ class XmlSerializationTest extends BaseSerializationTest
         $this->assertEquals($this->getContent('array_key_values'), $serializer->serialize(new ObjectWithXmlKeyValuePairs(), 'xml'));
     }
 
+    /**
+     * @param string $key
+     */
     protected function getContent($key)
     {
         if (!file_exists($file = __DIR__.'/xml/'.$key.'.xml')) {
@@ -130,4 +167,4 @@ class XmlSerializationTest extends BaseSerializationTest
     {
         return 'xml';
     }
-}
+}