Merge commit from fork

* Security Patch Control characters should not be allowed in protocol. * Tighten Up Drawing * Fix Test
oleibman · Jan 24, 2025 · cde2926 · cde2926
1 parent ed66270
commit cde2926
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 5 deletions.
diff --git a/src/PhpSpreadsheet/Worksheet/Drawing.php b/src/PhpSpreadsheet/Worksheet/Drawing.php
@@ -103,7 +103,7 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip
 
         $this->path = '';
         // Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
-        if (filter_var($path, FILTER_VALIDATE_URL)) {
+        if (filter_var($path, FILTER_VALIDATE_URL) || (preg_match('/^([\\w\\s\\x00-\\x1f]+):/u', $path) && !preg_match('/^([\\w]+):/u', $path))) {
             if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
                 throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
             }

diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php
@@ -1601,9 +1601,10 @@ private function generateRow(Worksheet $worksheet, array $values, int $row, stri
                 $url = $worksheet->getHyperlink($coordinate)->getUrl();
                 $urlDecode1 = html_entity_decode($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
                 $urlTrim = Preg::replace('/^\\s+/u', '', $urlDecode1);
-                $parseScheme = Preg::isMatch('/^([\\w\\s]+):/u', strtolower($urlTrim), $matches);
+                $parseScheme = Preg::isMatch('/^([\\w\\s\\x00-\\x1f]+):/u', strtolower($urlTrim), $matches);
                 if ($parseScheme && !in_array($matches[1], ['http', 'https', 'file', 'ftp', 'mailto', 's3'], true)) {
                     $cellData = htmlspecialchars($url, Settings::htmlEntityFlags());
+                    $cellData = self::replaceControlChars($cellData);
                 } else {
                     $tooltip = $worksheet->getHyperlink($coordinate)->getTooltip();
                     $tooltipOut = empty($tooltip) ? '' : (' title="' . htmlspecialchars($tooltip) . '"');
@@ -1658,6 +1659,20 @@ private function generateRow(Worksheet $worksheet, array $values, int $row, stri
         return $html;
     }
 
+    private static function replaceNonAscii(array $matches): string
+    {
+        return '&#' . mb_ord($matches[0], 'UTF-8') . ';';
+    }
+
+    private static function replaceControlChars(string $convert): string
+    {
+        return (string) preg_replace_callback(
+            '/[\\x00-\\x1f]/',
+            [self::class, 'replaceNonAscii'],
+            $convert
+        );
+    }
+
     /**
      * Takes array where of CSS properties / values and converts to CSS string.
      */

diff --git a/tests/PhpSpreadsheetTests/Reader/Html/HtmlImage2Test.php b/tests/PhpSpreadsheetTests/Reader/Html/HtmlImage2Test.php
@@ -6,6 +6,7 @@
 
 use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
 use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 class HtmlImage2Test extends TestCase
@@ -49,11 +50,11 @@ public function testCantInsertImageNotFound(): void
         self::assertCount(0, $drawingCollection);
     }
 
-    public function testCannotInsertImageBadProtocol(): void
+    #[DataProvider('providerBadProtocol')]
+    public function testCannotInsertImageBadProtocol(string $imagePath): void
     {
         $this->expectException(SpreadsheetException::class);
         $this->expectExceptionMessage('Invalid protocol for linked drawing');
-        $imagePath = 'httpx://phpspreadsheet.readthedocs.io/en/latest/topics/images/01-03-filter-icon-1.png';
         $html = '<table>
                     <tr>
                         <td><img src="' . $imagePath . '" alt="test image voilà"></td>
@@ -62,4 +63,17 @@ public function testCannotInsertImageBadProtocol(): void
         $filename = HtmlHelper::createHtml($html);
         HtmlHelper::loadHtmlIntoSpreadsheet($filename, true);
     }
+
+    public static function providerBadProtocol(): array
+    {
+        return [
+            'unknown protocol' => ['httpx://example.com/image.png'],
+            'embedded whitespace' => ['ht tp://example.com/image.png'],
+            'control character' => ["\x14http://example.com/image.png"],
+            'mailto' => ['mailto:[email protected]'],
+            'mailto whitespace' => ['mail to:[email protected]'],
+            'phar' => ['phar://example.com/image.phar'],
+            'phar control' => ["\x14phar://example.com/image.phar"],
+        ];
+    }
 }
diff --git a/tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php b/tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php
@@ -5,6 +5,7 @@
 namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
 
 use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PhpOffice\PhpSpreadsheet\Reader\Xml as XmlReader;
 use PhpOffice\PhpSpreadsheet\Writer\Html as HtmlWriter;
 use PHPUnit\Framework\TestCase;
 
@@ -17,7 +18,18 @@ public function testBadHyperlink(): void
         $spreadsheet = $reader->load($infile);
         $writer = new HtmlWriter($spreadsheet);
         $html = $writer->generateHtmlAll();
-        self::assertStringContainsString("<td class=\"column0 style1 f\">jav\tascript:alert()</td>", $html);
+        self::assertStringContainsString('<td class="column0 style1 f">jav&#9;ascript:alert()</td>', $html);
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testControlCharacter(): void
+    {
+        $reader = new XmlReader();
+        $infile = 'tests/data/Reader/Xml/sec-w24f.dontuse';
+        $spreadsheet = $reader->load($infile);
+        $writer = new HtmlWriter($spreadsheet);
+        $html = $writer->generateHtmlAll();
+        self::assertStringContainsString('<td class="column0 style0 f">&#20;j&#13;avascript:alert(1)</td>', $html);
         $spreadsheet->disconnectWorksheets();
     }
 }
diff --git a/tests/data/Reader/Xml/sec-w24f.dontuse b/tests/data/Reader/Xml/sec-w24f.dontuse
@@ -0,0 +1,65 @@
+<?xml version="1.0"?> 
+<?mso-application progid="Excel.Sheet"?> 
+<Workbook xmlns="urn:schemas-microsoft-com:office:spreadsheet" 
+ xmlns:o="urn:schemas-microsoft-com:office:office" 
+ xmlns:x="urn:schemas-microsoft-com:office:excel" 
+ xmlns:ss="urn:schemas-microsoft-com:office:spreadsheet" 
+ xmlns:html="http://www.w3.org/TR/REC-html40"> 
+ <DocumentProperties xmlns="urn:schemas-microsoft-com:office:office"> 
+  <Author>author</Author> 
+  <LastAuthor>author</LastAuthor> 
+  <Created>2015-06-05T18:19:34Z</Created> 
+  <LastSaved>2024-12-25T10:16:07Z</LastSaved> 
+  <Version>16.00</Version> 
+ </DocumentProperties> 
+ <OfficeDocumentSettings xmlns="urn:schemas-microsoft-com:office:office"> 
+  <AllowPNG/> 
+ </OfficeDocumentSettings> 
+ <ExcelWorkbook xmlns="urn:schemas-microsoft-com:office:excel"> 
+  <WindowHeight>11020</WindowHeight> 
+  <WindowWidth>19420</WindowWidth> 
+  <WindowTopX>32767</WindowTopX> 
+  <WindowTopY>32767</WindowTopY> 
+  <ProtectStructure>False</ProtectStructure> 
+  <ProtectWindows>False</ProtectWindows> 
+ </ExcelWorkbook> 
+ <Styles> 
+  <Style ss:ID="Default" ss:Name="Normal"> 
+   <Alignment ss:Vertical="Bottom"/> 
+   <Borders/> 
+   <Font ss:FontName="Calibri" x:Family="Swiss" ss:Size="11" ss:Color="#000000"/> 
+   <Interior/> 
+   <NumberFormat/> 
+   <Protection/> 
+  </Style> 
+  <Style ss:ID="s16"> 
+   <NumberFormat ss:Format="General Date"/> 
+  </Style> 
+ </Styles> 
+ <Worksheet ss:Name="Лист1"> 
+  <Table ss:ExpandedColumnCount="2" ss:ExpandedRowCount="6" x:FullColumns="1" 
+   x:FullRows="1" ss:DefaultRowHeight="14.5"> 
+   <Column ss:AutoFitWidth="0" ss:Width="194"/> 
+   <Row> 
+     <Cell ss:Formula="=HYPERLINK (CHAR(20) &amp; &quot;j&quot; &amp; CHAR(13) &amp; &quot;avascript:alert(1)&quot;)"><Data ss:Type="String"></Data></Cell> 
+   </Row> 
+  </Table> 
+  <WorksheetOptions xmlns="urn:schemas-microsoft-com:office:excel"> 
+   <PageSetup> 
+    <Header x:Margin="0.3"/> 
+    <Footer x:Margin="0.3"/> 
+    <PageMargins x:Bottom="0.75" x:Left="0.7" x:Right="0.7" x:Top="0.75"/> 
+   </PageSetup> 
+   <Selected/> 
+   <TopRowVisible>1</TopRowVisible> 
+   <Panes> 
+    <Pane> 
+     <Number>3</Number> 
+     <ActiveRow>6</ActiveRow> 
+    </Pane> 
+   </Panes> 
+   <ProtectObjects>False</ProtectObjects> 
+   <ProtectScenarios>False</ProtectScenarios> 
+  </WorksheetOptions> 
+ </Worksheet>
+</Workbook>