Merge pull request #107 from veewee/feature/disallow-doctype-4.x

Add disallow_doctype() configurator (no-DOCTYPE policy guard)

Author: veewee

docs/dom.md

@@ -480,6 +480,49 @@ Document::fromXmlFile(
 );
 ```
 
+#### disallow_doctype
+
+Rejects any document that carries a `<!DOCTYPE ...>` declaration by throwing a `VeeWee\Xml\Exception\DoctypeNotAllowedException`.
+It works on the parsed document, so it applies uniformly to every loader — string, file and node alike.
+
+```php
+use VeeWee\Xml\Dom\Document;
+use function VeeWee\Xml\Dom\Configurator\disallow_doctype;
+
+Document::fromXmlString(
+    $untrustedXml,
+    disallow_doctype()
+);
+```
+
+##### What this protects you from — and what it does not
+
+It is important to be honest about what this guard buys you, because most of the heavy lifting against
+[XML eXternal Entity (XXE)](https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing)
+attacks is already done by libxml itself.
+
+With the **default options** that this library passes to `Dom\XMLDocument` (i.e. you do *not* add libxml flags yourself), libxml already:
+
+* does **not** read external file entities (`<!ENTITY x SYSTEM "file:///etc/passwd">` is never resolved);
+* does **not** fetch external DTDs over the network (`<!DOCTYPE r SYSTEM "http://...">` triggers no request);
+* **aborts** entity-amplification ("billion laughs") attacks before expanding them.
+
+So on the default path you are already protected against the active XXE vectors, with or without this configurator.
+The only thing libxml still expands are small, non-amplifying *internal* entities — which is not an attack when the
+attacker already controls the whole document.
+
+What `disallow_doctype()` therefore adds is **policy and defence-in-depth**, not a fix for a libxml hole:
+
+* it enforces a clear, uniform "no DOCTYPE allowed" rule across all loaders, with a typed exception — useful when your
+  context forbids a DOCTYPE outright (e.g. SOAP / WS-Security / SAML);
+* it keeps you safe even if libxml's defaults ever change across versions, builds or `php.ini` settings.
+
+**What it does *not* protect against:** because it runs *after* parsing, it cannot undo anything that happened during
+parsing. If you knowingly (or unknowingly) hand the loader unsafe options such as `LIBXML_NOENT` or `LIBXML_DTDLOAD`,
+libxml *will* read the file / fetch the DTD while parsing — before this configurator ever runs. The rejection then comes
+too late. The takeaway is simple: **do not enable `LIBXML_NOENT` / `LIBXML_DTDLOAD` for untrusted input.** Keeping the
+default options is what actually keeps you safe; this configurator is the policy layer on top.
+
 #### document_uri
 
 Allows you to keep track of the document uri, even if you are using an in-memory string.

src/Xml/Dom/Configurator/disallow_doctype.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace VeeWee\Xml\Dom\Configurator;
+
+use Closure;
+use Dom\XMLDocument;
+use VeeWee\Xml\Exception\DoctypeNotAllowedException;
+
+/**
+ * Rejects any document that carries a `<!DOCTYPE ...>` declaration.
+ *
+ * Documents with a DOCTYPE can be abused for XML eXternal Entity (XXE) attacks,
+ * so loading untrusted XML through this configurator guards against that vector.
+ *
+ * @return Closure(XMLDocument): XMLDocument
+ */
+function disallow_doctype(): Closure
+{
+    return static function (XMLDocument $document): XMLDocument {
+        if ($document->doctype !== null) {
+            throw DoctypeNotAllowedException::create();
+        }
+
+        return $document;
+    };
+}

src/Xml/Exception/DoctypeNotAllowedException.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace VeeWee\Xml\Exception;
+
+use RuntimeException;
+
+final class DoctypeNotAllowedException extends RuntimeException implements ExceptionInterface
+{
+    private function __construct(string $message)
+    {
+        parent::__construct($message);
+    }
+
+    public static function create(): self
+    {
+        return new self(
+            'The XML document contains a DOCTYPE declaration, which is not allowed. '
+            . 'Documents with a DOCTYPE can be used for XML eXternal Entity (XXE) attacks.'
+        );
+    }
+}

src/bootstrap.php

@@ -24,6 +24,7 @@
         'Xml\Dom\Builder\xmlns_attributes' => __DIR__.'/Xml/Dom/Builder/xmlns_attributes.php',
         'Xml\Dom\Configurator\canonicalize' => __DIR__.'/Xml/Dom/Configurator/canonicalize.php',
         'Xml\Dom\Configurator\comparable' => __DIR__.'/Xml/Dom/Configurator/comparable.php',
+        'Xml\Dom\Configurator\disallow_doctype' => __DIR__.'/Xml/Dom/Configurator/disallow_doctype.php',
         'Xml\Dom\Configurator\document_uri' => __DIR__.'/Xml/Dom/Configurator/document_uri.php',
         'Xml\Dom\Configurator\format_output' => __DIR__.'/Xml/Dom/Configurator/format_output.php',
         'Xml\Dom\Configurator\normalize' => __DIR__.'/Xml/Dom/Configurator/normalize.php',

tests/Xml/Dom/Configurator/DisallowDoctypeTest.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace VeeWee\Tests\Xml\Dom\Configurator;
+
+use PHPUnit\Framework\TestCase;
+use VeeWee\Xml\Dom\Document;
+use VeeWee\Xml\Exception\DoctypeNotAllowedException;
+use function VeeWee\Xml\Dom\Configurator\disallow_doctype;
+use function VeeWee\Xml\Dom\Mapper\xml_string;
+
+final class DisallowDoctypeTest extends TestCase
+{
+    public function test_it_rejects_a_bare_doctype(): void
+    {
+        $this->expectException(DoctypeNotAllowedException::class);
+
+        Document::fromXmlString(
+            '<?xml version="1.0"?><!DOCTYPE r><r/>',
+            disallow_doctype()
+        );
+    }
+
+    public function test_it_rejects_an_external_file_entity_doctype(): void
+    {
+        $this->expectException(DoctypeNotAllowedException::class);
+
+        Document::fromXmlString(
+            '<?xml version="1.0"?><!DOCTYPE r [<!ENTITY x SYSTEM "file:///etc/hostname">]><r>&x;</r>',
+            disallow_doctype()
+        );
+    }
+
+    public function test_it_rejects_an_internal_entity_doctype(): void
+    {
+        $this->expectException(DoctypeNotAllowedException::class);
+
+        Document::fromXmlString(
+            '<?xml version="1.0"?><!DOCTYPE r ['
+            . '<!ENTITY a "aaaaaaaaaa">'
+            . '<!ENTITY b "&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;">'
+            . ']><r>&b;</r>',
+            disallow_doctype()
+        );
+    }
+
+    public function test_it_rejects_an_external_dtd_over_network(): void
+    {
+        $this->expectException(DoctypeNotAllowedException::class);
+
+        Document::fromXmlString(
+            '<?xml version="1.0"?><!DOCTYPE r SYSTEM "http://127.0.0.1:1/x.dtd"><r/>',
+            disallow_doctype()
+        );
+    }
+
+    public function test_it_allows_a_document_without_a_doctype(): void
+    {
+        $document = Document::fromXmlString(
+            '<?xml version="1.0"?><r>ok</r>',
+            disallow_doctype()
+        );
+
+        static::assertSame('<r>ok</r>', xml_string()($document->locateDocumentElement()));
+    }
+}