Skip to content

Commit cde2926

Browse files
oleibmanoleibman
authored
Merge commit from fork
* Security Patch Control characters should not be allowed in protocol. * Tighten Up Drawing * Fix Test
1 parent ed66270 commit cde2926

File tree

5 files changed

+111
-5
lines changed

5 files changed

+111
-5
lines changed

‎src/PhpSpreadsheet/Worksheet/Drawing.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip
103103

104104
$this->path = '';
105105
// Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
106-
if (filter_var($path, FILTER_VALIDATE_URL)) {
106+
if (filter_var($path, FILTER_VALIDATE_URL) || (preg_match('/^([\\w\\s\\x00-\\x1f]+):/u', $path) && !preg_match('/^([\\w]+):/u', $path))) {
107107
if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
108108
throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
109109
}

‎src/PhpSpreadsheet/Writer/Html.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1601,9 +1601,10 @@ private function generateRow(Worksheet $worksheet, array $values, int $row, stri
16011601
$url = $worksheet->getHyperlink($coordinate)->getUrl();
16021602
$urlDecode1 = html_entity_decode($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
16031603
$urlTrim = Preg::replace('/^\\s+/u', '', $urlDecode1);
1604-
$parseScheme = Preg::isMatch('/^([\\w\\s]+):/u', strtolower($urlTrim), $matches);
1604+
$parseScheme = Preg::isMatch('/^([\\w\\s\\x00-\\x1f]+):/u', strtolower($urlTrim), $matches);
16051605
if ($parseScheme && !in_array($matches[1], ['http', 'https', 'file', 'ftp', 'mailto', 's3'], true)) {
16061606
$cellData = htmlspecialchars($url, Settings::htmlEntityFlags());
1607+
$cellData = self::replaceControlChars($cellData);
16071608
} else {
16081609
$tooltip = $worksheet->getHyperlink($coordinate)->getTooltip();
16091610
$tooltipOut = empty($tooltip) ? '' : (' title="' . htmlspecialchars($tooltip) . '"');
@@ -1658,6 +1659,20 @@ private function generateRow(Worksheet $worksheet, array $values, int $row, stri
16581659
return $html;
16591660
}
16601661

1662+
private static function replaceNonAscii(array $matches): string
1663+
{
1664+
return '&#' . mb_ord($matches[0], 'UTF-8') . ';';
1665+
}
1666+
1667+
private static function replaceControlChars(string $convert): string
1668+
{
1669+
return (string) preg_replace_callback(
1670+
'/[\\x00-\\x1f]/',
1671+
[self::class, 'replaceNonAscii'],
1672+
$convert
1673+
);
1674+
}
1675+
16611676
/**
16621677
* Takes array where of CSS properties / values and converts to CSS string.
16631678
*/

‎tests/PhpSpreadsheetTests/Reader/Html/HtmlImage2Test.php

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
88
use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
9+
use PHPUnit\Framework\Attributes\DataProvider;
910
use PHPUnit\Framework\TestCase;
1011

1112
class HtmlImage2Test extends TestCase
@@ -49,11 +50,11 @@ public function testCantInsertImageNotFound(): void
4950
self::assertCount(0, $drawingCollection);
5051
}
5152

52-
public function testCannotInsertImageBadProtocol(): void
53+
#[DataProvider('providerBadProtocol')]
54+
public function testCannotInsertImageBadProtocol(string $imagePath): void
5355
{
5456
$this->expectException(SpreadsheetException::class);
5557
$this->expectExceptionMessage('Invalid protocol for linked drawing');
56-
$imagePath = 'httpx://phpspreadsheet.readthedocs.io/en/latest/topics/images/01-03-filter-icon-1.png';
5758
$html = '<table>
5859
<tr>
5960
<td><img src="' . $imagePath . '" alt="test image voilà"></td>
@@ -62,4 +63,17 @@ public function testCannotInsertImageBadProtocol(): void
6263
$filename = HtmlHelper::createHtml($html);
6364
HtmlHelper::loadHtmlIntoSpreadsheet($filename, true);
6465
}
66+
67+
public static function providerBadProtocol(): array
68+
{
69+
return [
70+
'unknown protocol' => ['httpx://example.com/image.png'],
71+
'embedded whitespace' => ['ht tp://example.com/image.png'],
72+
'control character' => ["\x14http://example.com/image.png"],
73+
'mailto' => ['mailto:xyz@example.com'],
74+
'mailto whitespace' => ['mail to:xyz@example.com'],
75+
'phar' => ['phar://example.com/image.phar'],
76+
'phar control' => ["\x14phar://example.com/image.phar"],
77+
];
78+
}
6579
}

‎tests/PhpSpreadsheetTests/Writer/Html/BadHyperlinkTest.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
66

77
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
8+
use PhpOffice\PhpSpreadsheet\Reader\Xml as XmlReader;
89
use PhpOffice\PhpSpreadsheet\Writer\Html as HtmlWriter;
910
use PHPUnit\Framework\TestCase;
1011

@@ -17,7 +18,18 @@ public function testBadHyperlink(): void
1718
$spreadsheet = $reader->load($infile);
1819
$writer = new HtmlWriter($spreadsheet);
1920
$html = $writer->generateHtmlAll();
20-
self::assertStringContainsString("<td class=\"column0 style1 f\">jav\tascript:alert()</td>", $html);
21+
self::assertStringContainsString('<td class="column0 style1 f">jav&#9;ascript:alert()</td>', $html);
22+
$spreadsheet->disconnectWorksheets();
23+
}
24+
25+
public function testControlCharacter(): void
26+
{
27+
$reader = new XmlReader();
28+
$infile = 'tests/data/Reader/Xml/sec-w24f.dontuse';
29+
$spreadsheet = $reader->load($infile);
30+
$writer = new HtmlWriter($spreadsheet);
31+
$html = $writer->generateHtmlAll();
32+
self::assertStringContainsString('<td class="column0 style0 f">&#20;j&#13;avascript:alert(1)</td>', $html);
2133
$spreadsheet->disconnectWorksheets();
2234
}
2335
}

‎tests/data/Reader/Xml/sec-w24f.dontuse

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?xml version="1.0"?>
2+
<?mso-application progid="Excel.Sheet"?>
3+
<Workbook xmlns="urn:schemas-microsoft-com:office:spreadsheet"
4+
xmlns:o="urn:schemas-microsoft-com:office:office"
5+
xmlns:x="urn:schemas-microsoft-com:office:excel"
6+
xmlns:ss="urn:schemas-microsoft-com:office:spreadsheet"
7+
xmlns:html="http://www.w3.org/TR/REC-html40">
8+
<DocumentProperties xmlns="urn:schemas-microsoft-com:office:office">
9+
<Author>author</Author>
10+
<LastAuthor>author</LastAuthor>
11+
<Created>2015-06-05T18:19:34Z</Created>
12+
<LastSaved>2024-12-25T10:16:07Z</LastSaved>
13+
<Version>16.00</Version>
14+
</DocumentProperties>
15+
<OfficeDocumentSettings xmlns="urn:schemas-microsoft-com:office:office">
16+
<AllowPNG/>
17+
</OfficeDocumentSettings>
18+
<ExcelWorkbook xmlns="urn:schemas-microsoft-com:office:excel">
19+
<WindowHeight>11020</WindowHeight>
20+
<WindowWidth>19420</WindowWidth>
21+
<WindowTopX>32767</WindowTopX>
22+
<WindowTopY>32767</WindowTopY>
23+
<ProtectStructure>False</ProtectStructure>
24+
<ProtectWindows>False</ProtectWindows>
25+
</ExcelWorkbook>
26+
<Styles>
27+
<Style ss:ID="Default" ss:Name="Normal">
28+
<Alignment ss:Vertical="Bottom"/>
29+
<Borders/>
30+
<Font ss:FontName="Calibri" x:Family="Swiss" ss:Size="11" ss:Color="#000000"/>
31+
<Interior/>
32+
<NumberFormat/>
33+
<Protection/>
34+
</Style>
35+
<Style ss:ID="s16">
36+
<NumberFormat ss:Format="General Date"/>
37+
</Style>
38+
</Styles>
39+
<Worksheet ss:Name="Лист1">
40+
<Table ss:ExpandedColumnCount="2" ss:ExpandedRowCount="6" x:FullColumns="1"
41+
x:FullRows="1" ss:DefaultRowHeight="14.5">
42+
<Column ss:AutoFitWidth="0" ss:Width="194"/>
43+
<Row>
44+
<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>
45+
</Row>
46+
</Table>
47+
<WorksheetOptions xmlns="urn:schemas-microsoft-com:office:excel">
48+
<PageSetup>
49+
<Header x:Margin="0.3"/>
50+
<Footer x:Margin="0.3"/>
51+
<PageMargins x:Bottom="0.75" x:Left="0.7" x:Right="0.7" x:Top="0.75"/>
52+
</PageSetup>
53+
<Selected/>
54+
<TopRowVisible>1</TopRowVisible>
55+
<Panes>
56+
<Pane>
57+
<Number>3</Number>
58+
<ActiveRow>6</ActiveRow>
59+
</Pane>
60+
</Panes>
61+
<ProtectObjects>False</ProtectObjects>
62+
<ProtectScenarios>False</ProtectScenarios>
63+
</WorksheetOptions>
64+
</Worksheet>
65+
</Workbook>

0 commit comments

Comments
 (0)