From bea2d4b30f24bcc8a7712e208d1359e603b45dda Mon Sep 17 00:00:00 2001
From: Adrien Crivelli <adrien.crivelli@gmail.com>
Date: Mon, 29 Jul 2024 16:19:27 +0900
Subject: [PATCH] Security: prevent XXE (XML External Entity) when loading
 files

Prevent XEE by hiding custom entities by using single quote to
declare a non-UTF-8 encoding.

XML standard, https://www.w3.org/TR/xml/#NT-EncodingDecl, allows single
quote to declare encoding, but we did not support it. Instead, we
incorrectly fell back on the default of UTF-8. That incorrectly kept the
XML as non-UTF-8, and thus prevented our regexp-based custom entity
detection mechanism to work.
---
 CHANGELOG.md                                  |  4 ++++
 .../Reader/Security/XmlScanner.php            | 24 ++++++++++++++-----
 .../Xml/XEETestInvalidUTF-7-single-quote.xml  |  2 ++
 .../Xml/XEETestValidUTF-8-single-quote.xml    |  4 ++++
 4 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 tests/data/Reader/Xml/XEETestInvalidUTF-7-single-quote.xml
 create mode 100644 tests/data/Reader/Xml/XEETestValidUTF-8-single-quote.xml

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 11aa4f80e..f5930d0f0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ## TBD - 3.0.0
 
+### Security Fix
+
+- Prevent XXE when loading files
+
 ### Added
 
 - Nothing
diff --git a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
index 979325a44..3946ed00e 100644
--- a/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
+++ b/src/PhpSpreadsheet/Reader/Security/XmlScanner.php
@@ -35,15 +35,11 @@ class XmlScanner
 
     private function toUtf8(string $xml): string
     {
-        $pattern = '/encoding="(.*?)"/';
-        $result = preg_match($pattern, $xml, $matches);
-        $charset = strtoupper($result ? $matches[1] : 'UTF-8');
-
+        $charset = $this->findCharSet($xml);
         if ($charset !== 'UTF-8') {
             $xml = self::forceString(mb_convert_encoding($xml, 'UTF-8', $charset));
 
-            $result = preg_match($pattern, $xml, $matches);
-            $charset = strtoupper($result ? $matches[1] : 'UTF-8');
+            $charset = $this->findCharSet($xml);
             if ($charset !== 'UTF-8') {
                 throw new Reader\Exception('Suspicious Double-encoded XML, spreadsheet file load() aborted to prevent XXE/XEE attacks');
             }
@@ -52,6 +48,22 @@ class XmlScanner
         return $xml;
     }
 
+    private function findCharSet(string $xml): string
+    {
+        $patterns = [
+            '/encoding="([^"]*]?)"/',
+            "/encoding='([^']*?)'/",
+        ];
+
+        foreach ($patterns as $pattern) {
+            if (preg_match($pattern, $xml, $matches)) {
+                return strtoupper($matches[1]);
+            }
+        }
+
+        return 'UTF-8';
+    }
+
     /**
      * Scan the XML for use of <!ENTITY to prevent XXE/XEE attacks.
      *
diff --git a/tests/data/Reader/Xml/XEETestInvalidUTF-7-single-quote.xml b/tests/data/Reader/Xml/XEETestInvalidUTF-7-single-quote.xml
new file mode 100644
index 000000000..9fc7eba72
--- /dev/null
+++ b/tests/data/Reader/Xml/XEETestInvalidUTF-7-single-quote.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding='UTF-7' standalone="yes"?>
+    +ADw-+ACE-DOCTYPE+ACA-foo+ACA-+AFs-+ADw-+ACE-ENTITY+ACA-toreplace+ACA-+ACI-xxe+AF8-test+ACI-+AD4-+ACA-+AF0-+AD4-+AAo-+ADw-sst+ACA-xmlns+AD0-+ACI-http://schemas.openxmlformats.org/spreadsheetml/2006/main+ACI-+ACA-count+AD0-+ACI-2+ACI-+ACA-uniqueCount+AD0-+ACI-1+ACI-+AD4-+ADw-si+AD4-+ADw-t+AD4-+ACY-toreplace+ADs-+ADw-/t+AD4-+ADw-/si+AD4-+ADw-/sst+AD4-
diff --git a/tests/data/Reader/Xml/XEETestValidUTF-8-single-quote.xml b/tests/data/Reader/Xml/XEETestValidUTF-8-single-quote.xml
new file mode 100644
index 000000000..e478c7d40
--- /dev/null
+++ b/tests/data/Reader/Xml/XEETestValidUTF-8-single-quote.xml
@@ -0,0 +1,4 @@
+<?xml version='1.0' encoding='UTF-8' standalone='yes'?>
+<root>
+	test: Valid
+</root>