diff --git a/src/PhpSpreadsheet/Reader/Gnumeric.php b/src/PhpSpreadsheet/Reader/Gnumeric.php index a726599e..8837e7da 100644 --- a/src/PhpSpreadsheet/Reader/Gnumeric.php +++ b/src/PhpSpreadsheet/Reader/Gnumeric.php @@ -43,7 +43,7 @@ class Gnumeric extends BaseReader { $this->readFilter = new DefaultReadFilter(); $this->referenceHelper = ReferenceHelper::getInstance(); - $this->securityScanner = new XmlScanner(); + $this->securityScanner = XmlScanner::getInstance($this); } /** diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php index fc09f1be..e6543592 100644 --- a/src/PhpSpreadsheet/Reader/Html.php +++ b/src/PhpSpreadsheet/Reader/Html.php @@ -111,7 +111,7 @@ class Html extends BaseReader public function __construct() { $this->readFilter = new DefaultReadFilter(); - $this->securityScanner = new XmlScanner('securityScanner = XmlScanner::getInstance($this); } /** diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 7b25901e..80fab980 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -31,7 +31,7 @@ class Ods extends BaseReader public function __construct() { $this->readFilter = new DefaultReadFilter(); - $this->securityScanner = new XmlScanner(); + $this->securityScanner = XmlScanner::getInstance($this); } /** diff --git a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php index eefd7c7d..36710209 100644 --- a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php +++ b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php @@ -2,7 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Reader\Security; -use PhpOffice\PhpSpreadsheet\Reader\Exception; +use PhpOffice\PhpSpreadsheet\Reader; class XmlScanner { @@ -13,15 +13,38 @@ class XmlScanner */ private $libxmlDisableEntityLoader = false; + private $previousLibxmlDisableEntityLoaderValue; private $pattern; - public function __construct($pattern = 'pattern = $pattern; $this->libxmlDisableEntityLoader = $this->identifyLibxmlDisableEntityLoaderAvailability(); 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('pattern)) . '\\0?/'; 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; @@ -76,7 +99,7 @@ class XmlScanner * * @param string $filestream * - * @throws Exception + * @throws Reader\Exception * * @return string */ diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index b1e076a8..806a6d4f 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -59,7 +59,7 @@ class Xlsx extends BaseReader { $this->readFilter = new DefaultReadFilter(); $this->referenceHelper = ReferenceHelper::getInstance(); - $this->securityScanner = new XmlScanner(); + $this->securityScanner = XmlScanner::getInstance($this); } /** diff --git a/src/PhpSpreadsheet/Reader/Xml.php b/src/PhpSpreadsheet/Reader/Xml.php index 9e982f2a..3781c427 100644 --- a/src/PhpSpreadsheet/Reader/Xml.php +++ b/src/PhpSpreadsheet/Reader/Xml.php @@ -47,7 +47,7 @@ class Xml extends BaseReader public function __construct() { $this->readFilter = new DefaultReadFilter(); - $this->securityScanner = new XmlScanner(); + $this->securityScanner = XmlScanner::getInstance($this); } /** diff --git a/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php b/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php index b914de08..8eaa56f8 100644 --- a/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php @@ -15,7 +15,7 @@ class XmlScannerTest extends TestCase */ public function testValidXML($filename, $expectedResult) { - $reader = new XmlScanner(); + $reader = XmlScanner::getInstance(new \PhpOffice\PhpSpreadsheet\Reader\Xml()); $result = $reader->scanFile($filename); self::assertEquals($expectedResult, $result); } @@ -39,7 +39,7 @@ class XmlScannerTest extends TestCase { $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'; $result = $reader->scanFile($filename); self::assertEquals($expectedResult, $result);