Use factory for XMLcanner

This commit is contained in:
MarkBaker 2018-11-23 23:05:17 +01:00
parent dff1151369
commit abad49d426
7 changed files with 36 additions and 13 deletions

View File

@ -43,7 +43,7 @@ class Gnumeric extends BaseReader
{ {
$this->readFilter = new DefaultReadFilter(); $this->readFilter = new DefaultReadFilter();
$this->referenceHelper = ReferenceHelper::getInstance(); $this->referenceHelper = ReferenceHelper::getInstance();
$this->securityScanner = new XmlScanner(); $this->securityScanner = XmlScanner::getInstance($this);
} }
/** /**

View File

@ -111,7 +111,7 @@ class Html extends BaseReader
public function __construct() public function __construct()
{ {
$this->readFilter = new DefaultReadFilter(); $this->readFilter = new DefaultReadFilter();
$this->securityScanner = new XmlScanner('<!ENTITY'); $this->securityScanner = XmlScanner::getInstance($this);
} }
/** /**

View File

@ -31,7 +31,7 @@ class Ods extends BaseReader
public function __construct() public function __construct()
{ {
$this->readFilter = new DefaultReadFilter(); $this->readFilter = new DefaultReadFilter();
$this->securityScanner = new XmlScanner(); $this->securityScanner = XmlScanner::getInstance($this);
} }
/** /**

View File

@ -2,7 +2,7 @@
namespace PhpOffice\PhpSpreadsheet\Reader\Security; namespace PhpOffice\PhpSpreadsheet\Reader\Security;
use PhpOffice\PhpSpreadsheet\Reader\Exception; use PhpOffice\PhpSpreadsheet\Reader;
class XmlScanner class XmlScanner
{ {
@ -13,15 +13,38 @@ class XmlScanner
*/ */
private $libxmlDisableEntityLoader = false; private $libxmlDisableEntityLoader = false;
private $previousLibxmlDisableEntityLoaderValue;
private $pattern; private $pattern;
public function __construct($pattern = '<!DOCTYPE') private function __construct($pattern = '<!DOCTYPE')
{ {
$this->pattern = $pattern; $this->pattern = $pattern;
$this->libxmlDisableEntityLoader = $this->identifyLibxmlDisableEntityLoaderAvailability(); $this->libxmlDisableEntityLoader = $this->identifyLibxmlDisableEntityLoaderAvailability();
if ($this->libxmlDisableEntityLoader) { if ($this->libxmlDisableEntityLoader) {
libxml_disable_entity_loader(true); $this->previousLibxmlDisableEntityLoaderValue = libxml_disable_entity_loader(true);
}
}
public function __destruct()
{
if ($this->libxmlDisableEntityLoader) {
libxml_disable_entity_loader($this->previousLibxmlDisableEntityLoaderValue);
}
}
public static function getInstance(Reader\IReader $reader)
{
switch(true) {
case $reader instanceof Reader\Html:
return new self('<!ENTITY');
case $reader instanceof Reader\Xlsx:
case $reader instanceof Reader\Xml:
case $reader instanceof Reader\Ods:
case $reader instanceof Reader\Gnumeric:
return new self('<!DOCTYPE');
default:
return new self('<!DOCTYPE');
} }
} }
@ -48,7 +71,7 @@ class XmlScanner
* *
* @param mixed $xml * @param mixed $xml
* *
* @throws Exception * @throws Reader\Exception
* *
* @return string * @return string
*/ */
@ -65,7 +88,7 @@ class XmlScanner
// Don't rely purely on libxml_disable_entity_loader() // Don't rely purely on libxml_disable_entity_loader()
$pattern = '/\\0?' . implode('\\0?', str_split($this->pattern)) . '\\0?/'; $pattern = '/\\0?' . implode('\\0?', str_split($this->pattern)) . '\\0?/';
if (preg_match($pattern, $xml)) { if (preg_match($pattern, $xml)) {
throw new Exception('Detected use of ENTITY in XML, spreadsheet file load() aborted to prevent XXE/XEE attacks'); throw new Reader\Exception('Detected use of ENTITY in XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
} }
return $xml; return $xml;
@ -76,7 +99,7 @@ class XmlScanner
* *
* @param string $filestream * @param string $filestream
* *
* @throws Exception * @throws Reader\Exception
* *
* @return string * @return string
*/ */

View File

@ -59,7 +59,7 @@ class Xlsx extends BaseReader
{ {
$this->readFilter = new DefaultReadFilter(); $this->readFilter = new DefaultReadFilter();
$this->referenceHelper = ReferenceHelper::getInstance(); $this->referenceHelper = ReferenceHelper::getInstance();
$this->securityScanner = new XmlScanner(); $this->securityScanner = XmlScanner::getInstance($this);
} }
/** /**

View File

@ -47,7 +47,7 @@ class Xml extends BaseReader
public function __construct() public function __construct()
{ {
$this->readFilter = new DefaultReadFilter(); $this->readFilter = new DefaultReadFilter();
$this->securityScanner = new XmlScanner(); $this->securityScanner = XmlScanner::getInstance($this);
} }
/** /**

View File

@ -15,7 +15,7 @@ class XmlScannerTest extends TestCase
*/ */
public function testValidXML($filename, $expectedResult) public function testValidXML($filename, $expectedResult)
{ {
$reader = new XmlScanner(); $reader = XmlScanner::getInstance(new \PhpOffice\PhpSpreadsheet\Reader\Xml());
$result = $reader->scanFile($filename); $result = $reader->scanFile($filename);
self::assertEquals($expectedResult, $result); self::assertEquals($expectedResult, $result);
} }
@ -39,7 +39,7 @@ class XmlScannerTest extends TestCase
{ {
$this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class);
$reader = new XmlScanner(); $reader = XmlScanner::getInstance(new \PhpOffice\PhpSpreadsheet\Reader\Xml());
$expectedResult = 'FAILURE: Should throw an Exception rather than return a value'; $expectedResult = 'FAILURE: Should throw an Exception rather than return a value';
$result = $reader->scanFile($filename); $result = $reader->scanFile($filename);
self::assertEquals($expectedResult, $result); self::assertEquals($expectedResult, $result);