Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history
* Explicitly disable loading of external entities

* Remove problematic options

* Move test for dangerous XML before where the document is loaded

* Defense in depth: test for DOCTYPE in two different ways

* Add unit test

* Use defined() for LIB_NO_XXE
  • Loading branch information
tvdijen authored Dec 1, 2024
1 parent f32fdf3 commit 5fd4ce4
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
14 changes: 13 additions & 1 deletion src/SAML2/DOMDocumentFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,27 @@ public static function fromString(string $xml) : DOMDocument
{
if (trim($xml) === '') {
throw InvalidArgumentException::invalidType('non-empty string', $xml);
} elseif (preg_match('/<(\s*)!(\s*)DOCTYPE/', $xml)) {
throw new RuntimeException(
'Dangerous XML detected, DOCTYPE nodes are not allowed in the XML body'
);
} elseif (PHP_VERSION_ID < 80000) {
$entityLoader = libxml_disable_entity_loader(true);
} else {
libxml_set_external_entity_loader(null);
}

$internalErrors = libxml_use_internal_errors(true);
libxml_clear_errors();

$domDocument = self::create();
$options = LIBXML_DTDLOAD | LIBXML_DTDATTR | LIBXML_NONET | LIBXML_PARSEHUGE;
$options = LIBXML_NONET | LIBXML_PARSEHUGE;

/* LIBXML_NO_XXE available from PHP 8.4 */
if (defined('LIBXML_NO_XXE')) {
$options |= LIBXML_NO_XXE;
}

if (defined('LIBXML_COMPACT')) {
$options |= LIBXML_COMPACT;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/SAML2/DOMDocumentFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,22 @@ public function testStringThatContainsDocTypeIsNotAccepted() : void
}


/**
* @group domdocument
* @return void
*/
public function testStringThatContainsDocTypeIsNotAccepted2(): void
{
$xml = '<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE foo [<!ENTITY % exfiltrate SYSTEM "file://dev/random">%exfiltrate;]>
<foo>y</foo>';
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage(
'Dangerous XML detected, DOCTYPE nodes are not allowed in the XML body',
);
DOMDocumentFactory::fromString($xml);
}

/**
* @group domdocument
* @return void
Expand Down

0 comments on commit 5fd4ce4

Please sign in to comment.