Skip to content
This repository was archived by the owner on Jan 21, 2020. It is now read-only.

Add a way to customize the load command to run loadHTML or pass addition... #6

Closed
wants to merge 2 commits into from
Closed

Add a way to customize the load command to run loadHTML or pass addition... #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb82fe9
add a way to customize the load command to run loadHTML or pass addit…
Oct 3, 2014
7a3deb3
skip html test if a too low libxml version is detected
Dec 16, 2014
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
10 changes: 8 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,25 @@ php:
- 5.5
- 5.6
- hhvm

matrix:
allow_failures:
- php: hhvm

before_install:
# need to update libxml to 2.7.8 to be able to run tests using the
# LIBXML_HTML_NODEFDTD and LIBXML_HTML_NOIMPLIED libxml constant
Copy link
Member

Choose a reason for hiding this comment

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

What if those flags are not existing at all? Just a hard failure in the test suite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ATM yes, I'll add a check for libxml version and skip the test like mentioned in the phpunit docs, sec ;)

if (!extension_loaded('mysqli')) {
            $this->markTestSkipped(
              'The MySQLi extension is not available.'
            );
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- sudo apt-get update
- sudo apt-get -o DPkg::Options::="--force-confold" -y upgrade

before_script:
- composer self-update
- composer install --dev

script:
- ./vendor/bin/phpunit -c ./tests
- ./vendor/bin/phpcs --standard=PSR2 --ignore=tests/Bootstrap.php library tests

notifications:
irc: "irc.freenode.org#zftalk.dev"
email: false
42 changes: 40 additions & 2 deletions library/ZendXml/Security.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ protected static function heuristicScan($xml)
*
* @param string $xml
* @param DomDocument $dom
* @param int $libXmlConstants additional libxml constants to pass in
* @param Callable $callback the callback to use to create the dom element
* @throws Exception\RuntimeException
* @return SimpleXMLElement|DomDocument|boolean
*/
public static function scan($xml, DOMDocument $dom = null)
private static function scanString($xml, $dom, $libXmlConstants, $callback)
{
// If running with PHP-FPM we perform an heuristic scan
// We cannot use libxml_disable_entity_loader because of this bug
Expand All @@ -63,7 +65,9 @@ public static function scan($xml, DOMDocument $dom = null)
}
return false;
}, E_WARNING);
$result = $dom->loadXml($xml, LIBXML_NONET);

$result = $callback($xml, $dom, LIBXML_NONET | $libXmlConstants);

restore_error_handler();

// Entity load to previous setting
Expand Down Expand Up @@ -97,6 +101,40 @@ public static function scan($xml, DOMDocument $dom = null)
return $dom;
}

/**
* Scan HTML string for potential XXE and XEE attacks
*
* @param string $xml
* @param DomDocument $dom
* @param int $libXmlConstants additional libxml constants to pass in
* @throws Exception\RuntimeException
* @return SimpleXMLElement|DomDocument|boolean
*/
public static function scanHtml($html, DOMDocument $dom = null, $libXmlConstants = 0)
{
$callback = function ($html, $dom, $constants) {
return $dom->loadHtml($html, $constants);
};
return self::scanString($html, $dom, $libXmlConstants, $callback);
}

/**
* Scan XML string for potential XXE and XEE attacks
*
* @param string $xml
* @param DomDocument $dom
* @param int $libXmlConstants additional libxml constants to pass in
* @throws Exception\RuntimeException
* @return SimpleXMLElement|DomDocument|boolean
*/
public static function scan($xml, DOMDocument $dom = null, $libXmlConstants = 0)
{
$callback = function ($xml, $dom, $constants) {
return $dom->loadXml($xml, $constants);
};
return self::scanString($xml, $dom, $libXmlConstants, $callback);
}

/**
* Scan XML file for potential XXE/XEE attacks
*
Expand Down
25 changes: 25 additions & 0 deletions tests/ZendXmlTest/SecurityTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,31 @@ public function testScanDom()
$this->assertEquals($node->nodeValue, 'test');
}

/**
* @requires PHP 5.4
Copy link
Member

Choose a reason for hiding this comment

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

What about PHP 5.3? Can there be a test for it as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

5.3 does not allow any parameters to be passed to loadHtml so LIBXML_NONET can not be passed in, see http://php.net/manual/de/domdocument.loadhtml.php

I'm unsure if this can be fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Heh, we need 5.3 compatibility here for now :-\

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only option I see atm is to fall back to an xml parser in case php 5.3 and scanHtml is being used. This might yield weird results though so I'm unsure if this is the way to go. Or maybe throw an exception if its used on PHP 5.3 and tell the user to use loadXml instead?

Copy link
Member

Choose a reason for hiding this comment

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

@ezimuel ping on this.

Or maybe throw an exception if its used on PHP 5.3 and tell the user to use loadXml instead?

Can't really do, as there are parts of the codebase loading DOM nodes that are indeed used also in PHP 5.3 context. Writing an HTML parser is not attracting either, and would just be another huge source of trouble.

*/
public function testScanDomHTML()
{
// loadHtml accepts constants in php >= 5.4
// http://php.net/manual/de/domdocument.loadhtml.php
// LIBXML_HTML_NODEFDTD and LIBXML_HTML_NOIMPLIED require libxml 2.7.8+
// http://php.net/manual/de/libxml.constants.php
if (version_compare(LIBXML_DOTTED_VERSION, '2.7.8', '<')) {
$this->markTestSkipped(
'libxml 2.7.8+ required but found ' . LIBXML_DOTTED_VERSION
);
}

$dom = new DOMDocument('1.0');
$html = <<<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()
{
$xml = <<<XML
Expand Down