Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a small PHP 8.0 fix #3

Merged
merged 8 commits into from
Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/.phpunit.result.cache
/clover.xml
/composer.lock
/coveralls-upload.json
Expand Down
21 changes: 11 additions & 10 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
bootstrap="vendor/autoload.php"
colors="true">
<phpunit
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the local XML schema definition, otherwise we have to update it with each version..

bootstrap="vendor/autoload.php"
colors="true">
<coverage processUncoveredFiles="true">
<include>
<directory suffix=".php">./src</directory>
</include>
</coverage>

<testsuites>
<testsuite name="laminas-xml Test Suite">
<directory>./test</directory>
</testsuite>
</testsuites>

<filter>
<whitelist processUncoveredFilesFromWhitelist="true">
<directory suffix=".php">./src</directory>
</whitelist>
</filter>
</phpunit>
72 changes: 41 additions & 31 deletions test/MultibyteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
namespace LaminasTest\Xml;

use Laminas\Xml\Exception;
use Laminas\Xml\Exception\RuntimeException;
use Laminas\Xml\Security;
use PHPUnit\Framework\TestCase;
use ReflectionMethod;

Expand All @@ -17,7 +19,10 @@
*/
class MultibyteTest extends TestCase
{
public function multibyteEncodings()
/**
* @psalm-return array<array-key, array{0: string, 1: string, 2: int}>
*/
public function multibyteEncodings(): array
{
return [
'UTF-16LE' => ['UTF-16LE', pack('CC', 0xff, 0xfe), 3],
Expand All @@ -27,76 +32,79 @@ public function multibyteEncodings()
];
}

public function getXmlWithXXE()
public function getXmlWithXXE(): string
{
return <<<XML
<?xml version="1.0" encoding="{ENCODING}"?>
<!DOCTYPE methodCall [
<!ENTITY pocdata SYSTEM "file:///etc/passwd">
]>
<methodCall>
<methodName>retrieved: &pocdata;</methodName>
</methodCall>
XML;
<?xml version="1.0" encoding="{ENCODING}"?>
<!DOCTYPE methodCall [
<!ENTITY pocdata SYSTEM "file:///etc/passwd">
]>
<methodCall>
<methodName>retrieved: &pocdata;</methodName>
</methodCall>
XML;
Comment on lines +38 to +45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note, in case anybody wonders why I did this: PHP 7.3 adds the ability to indent heredocs and nowdocs. In each case, any leading whitespace before the closing marker is considered indentation, and stripped from each line of the string. This makes them far more readable when declared in indented code, which was the whole point of the change.

}

/**
* Invoke Laminas\Xml\Security::heuristicScan with the provided XML.
*
* @param string $xml
* @return void
* @throws Exception\RuntimeException
*/
public function invokeHeuristicScan($xml)
public function invokeHeuristicScan(string $xml): void
{
$r = new ReflectionMethod('Laminas\Xml\Security', 'heuristicScan');
$r = new ReflectionMethod(Security::class, 'heuristicScan');
$r->setAccessible(true);
return $r->invoke(null, $xml);
$r->invoke(null, $xml);
}

/**
* @dataProvider multibyteEncodings
* @group heuristicDetection
*/
public function testDetectsMultibyteXXEVectorsUnderFPMWithEncodedStringMissingBOM($encoding, $bom, $bomLength)
{
public function testDetectsMultibyteXXEVectorsUnderFPMWithEncodedStringMissingBOM(
string $encoding,
string $bom,
int $bomLength
): void {
$xml = $this->getXmlWithXXE();
$xml = str_replace('{ENCODING}', $encoding, $xml);
$xml = iconv('UTF-8', $encoding, $xml);
$this->assertNotSame(0, strncmp($xml, $bom, $bomLength));
$this->expectException('Laminas\Xml\Exception\RuntimeException');
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('ENTITY');
$this->invokeHeuristicScan($xml);
}

/**
* @dataProvider multibyteEncodings
*/
public function testDetectsMultibyteXXEVectorsUnderFPMWithEncodedStringUsingBOM($encoding, $bom)
{
public function testDetectsMultibyteXXEVectorsUnderFPMWithEncodedStringUsingBOM(
string $encoding,
string $bom
): void {
$xml = $this->getXmlWithXXE();
$xml = str_replace('{ENCODING}', $encoding, $xml);
$orig = iconv('UTF-8', $encoding, $xml);
$xml = $bom . $orig;
$this->expectException('Laminas\Xml\Exception\RuntimeException');
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('ENTITY');
$this->invokeHeuristicScan($xml);
}

public function getXmlWithoutXXE()
public function getXmlWithoutXXE(): string
{
return <<<XML
<?xml version="1.0" encoding="{ENCODING}"?>
<methodCall>
<methodName>retrieved: &pocdata;</methodName>
</methodCall>
XML;
<?xml version="1.0" encoding="{ENCODING}"?>
<methodCall>
<methodName>retrieved: &pocdata;</methodName>
</methodCall>
XML;
}

/**
* @dataProvider multibyteEncodings
*/
public function testDoesNotFlagValidMultibyteXmlAsInvalidUnderFPM($encoding)
public function testDoesNotFlagValidMultibyteXmlAsInvalidUnderFPM(string $encoding): void
{
$xml = $this->getXmlWithoutXXE();
$xml = str_replace('{ENCODING}', $encoding, $xml);
Expand All @@ -113,13 +121,15 @@ public function testDoesNotFlagValidMultibyteXmlAsInvalidUnderFPM($encoding)
* @dataProvider multibyteEncodings
* @group mixedEncoding
*/
public function testDetectsXXEWhenXMLDocumentEncodingDiffersFromFileEncoding($encoding, $bom)
{
public function testDetectsXXEWhenXMLDocumentEncodingDiffersFromFileEncoding(
string $encoding,
string $bom
): void {
$xml = $this->getXmlWithXXE();
$xml = str_replace('{ENCODING}', 'UTF-8', $xml);
$xml = iconv('UTF-8', $encoding, $xml);
$xml = $bom . $xml;
$this->expectException('Laminas\Xml\Exception\RuntimeException');
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('ENTITY');
$this->invokeHeuristicScan($xml);
}
Expand Down
94 changes: 47 additions & 47 deletions test/SecurityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,52 @@ class SecurityTest extends TestCase
/**
* @expectedException Laminas\Xml\Exception\RuntimeException
*/
public function testScanForXEE()
public function testScanForXEE(): void
{
$xml = <<<XML
<?xml version="1.0"?>
<!DOCTYPE results [<!ENTITY harmless "completely harmless">]>
<results>
<result>This result is &harmless;</result>
</results>
XML;
<?xml version="1.0"?>
<!DOCTYPE results [<!ENTITY harmless "completely harmless">]>
<results>
<result>This result is &harmless;</result>
</results>
XML;

$this->expectException('Laminas\Xml\Exception\RuntimeException');
$result = XmlSecurity::scan($xml);
XmlSecurity::scan($xml);
}

public function testScanForXXE()
public function testScanForXXE(): void
{
$file = tempnam(sys_get_temp_dir(), 'laminas-xml_Security');
file_put_contents($file, 'This is a remote content!');
$xml = <<<XML
<?xml version="1.0"?>
<!DOCTYPE root
[
<!ENTITY foo SYSTEM "file://$file">
]>
<results>
<result>&foo;</result>
</results>
XML;
<?xml version="1.0"?>
<!DOCTYPE root
[
<!ENTITY foo SYSTEM "file://$file">
]>
<results>
<result>&foo;</result>
</results>
XML;

try {
$result = XmlSecurity::scan($xml);
XmlSecurity::scan($xml);
} catch (Exception\RuntimeException $e) {
unlink($file);
return;
}
$this->fail('An expected exception has not been raised.');
}

public function testScanSimpleXmlResult()
public function testScanSimpleXmlResult(): void
{
$result = XmlSecurity::scan($this->getXml());
$this->assertTrue($result instanceof SimpleXMLElement);
$this->assertEquals($result->result, 'test');
}

public function testScanDom()
public function testScanDom(): void
{
$dom = new DOMDocument('1.0');
$result = XmlSecurity::scan($this->getXml(), $dom);
Expand All @@ -73,7 +73,7 @@ public function testScanDom()
$this->assertEquals($node->nodeValue, 'test');
}

public function testScanDomHTML()
public function testScanDomHTML(): void
{
// LIBXML_HTML_NODEFDTD and LIBXML_HTML_NOIMPLIED require libxml 2.7.8+
// http://php.net/manual/de/libxml.constants.php
Expand All @@ -85,36 +85,36 @@ public function testScanDomHTML()

$dom = new DOMDocument('1.0');
$html = <<<HTML
<p>a simple test</p>
HTML;
<p>a simple test</p>
HTML;
$constants = LIBXML_HTML_NODEFDTD | LIBXML_HTML_NOIMPLIED;
$result = XmlSecurity::scanHtml($html, $dom, $constants);
$this->assertTrue($result instanceof DOMDocument);
$this->assertEquals($html, trim($result->saveHtml()));
}

public function testScanInvalidXml()
public function testScanInvalidXml(): void
{
$xml = <<<XML
<foo>test</bar>
XML;
<foo>test</bar>
XML;

$result = XmlSecurity::scan($xml);
$this->assertFalse($result);
}

public function testScanInvalidXmlDom()
public function testScanInvalidXmlDom(): void
{
$xml = <<<XML
<foo>test</bar>
XML;
<foo>test</bar>
XML;

$dom = new DOMDocument('1.0');
$result = XmlSecurity::scan($xml, $dom);
$this->assertFalse($result);
}

public function testScanFile()
public function testScanFile(): void
{
$file = tempnam(sys_get_temp_dir(), 'laminas-xml_Security');
file_put_contents($file, $this->getXml());
Expand All @@ -125,32 +125,32 @@ public function testScanFile()
unlink($file);
}

public function testScanXmlWithDTD()
public function testScanXmlWithDTD(): void
{
$xml = <<<XML
<?xml version="1.0"?>
<!DOCTYPE results [
<!ELEMENT results (result+)>
<!ELEMENT result (#PCDATA)>
]>
<results>
<result>test</result>
</results>
XML;
<?xml version="1.0"?>
<!DOCTYPE results [
<!ELEMENT results (result+)>
<!ELEMENT result (#PCDATA)>
]>
<results>
<result>test</result>
</results>
XML;

$dom = new DOMDocument('1.0');
$result = XmlSecurity::scan($xml, $dom);
$this->assertTrue($result instanceof DOMDocument);
$this->assertTrue($result->validate());
}

protected function getXml()
protected function getXml(): string
{
return <<<XML
<?xml version="1.0"?>
<results>
<result>test</result>
</results>
XML;
<?xml version="1.0"?>
<results>
<result>test</result>
</results>
XML;
}
}