Skip to content

Commit

Permalink
Backport Security Patch
Browse files Browse the repository at this point in the history
  • Loading branch information
@oleibman
oleibman committed Sep 24, 2024
1 parent 9c90ed6 commit d95bc29
Show file tree
Hide file tree
Showing 13 changed files with 214 additions and 42 deletions.
2 changes: 1 addition & 1 deletion phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ parameters:

-
message: "#^Offset 'mime' does not exist on array\\{\\}\\|array\\{0\\: int\\<0, max\\>, 1\\: int\\<0, max\\>, 2\\: int, 3\\: string, mime\\: string, channels\\?\\: int, bits\\?\\: int\\}\\.$#"
count: 2
count: 1
path: src/PhpSpreadsheet/Writer/Html.php

-
Expand Down
14 changes: 11 additions & 3 deletions src/PhpSpreadsheet/Reader/Xlsx.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1291,7 +1291,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$hfImages[$shapeId]->setName((string) $imageData['title']);
}

$hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false);
$hfImages[$shapeId]->setPath('zip://' . File::realpath($filename) . '#' . $drawings[(string) $imageData['relid']], false, $zip);
$hfImages[$shapeId]->setResizeProportional(false);
$hfImages[$shapeId]->setWidth($style['width']);
$hfImages[$shapeId]->setHeight($style['height']);
Expand Down Expand Up @@ -1401,7 +1401,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$objDrawing->setPath(
'zip://' . File::realpath($filename) . '#' .
$images[$embedImageKey],
false
false,
$zip
);
} else {
$linkImageKey = (string) self::getArrayItem(
Expand All @@ -1412,6 +1413,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
$objDrawing->setPath($url);
}
if ($objDrawing->getPath() === '') {
continue;
}
}
$objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $oneCellAnchor->from->col) + 1) . ($oneCellAnchor->from->row + 1));

Expand Down Expand Up @@ -1486,7 +1490,8 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$objDrawing->setPath(
'zip://' . File::realpath($filename) . '#' .
$images[$embedImageKey],
false
false,
$zip
);
} else {
$linkImageKey = (string) self::getArrayItem(
Expand All @@ -1497,6 +1502,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$url = str_replace('xl/drawings/', '', $images[$linkImageKey]);
$objDrawing->setPath($url);
}
if ($objDrawing->getPath() === '') {
continue;
}
}
$objDrawing->setCoordinates(Coordinate::stringFromColumnIndex(((int) $twoCellAnchor->from->col) + 1) . ($twoCellAnchor->from->row + 1));

Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Worksheet/BaseDrawing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public function setWorksheet(?Worksheet $worksheet = null, bool $overrideOld = f
{
if ($this->worksheet === null) {
// Add drawing to \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet
if ($worksheet !== null) {
if ($worksheet !== null && !($this instanceof Drawing && $this->getPath() === '')) {
$this->worksheet = $worksheet;
$this->worksheet->getCell($this->coordinates);
$this->worksheet->getDrawingCollection()->append($this);
Expand Down
70 changes: 51 additions & 19 deletions src/PhpSpreadsheet/Worksheet/Drawing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,40 +106,72 @@ public function getPath()
*/
public function setPath($path, $verifyFile = true, $zip = null)
{
if ($verifyFile && preg_match('~^data:image/[a-z]+;base64,~', $path) !== 1) {
// Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
if (filter_var($path, FILTER_VALIDATE_URL)) {
$this->path = $path;
// Implicit that it is a URL, rather store info than running check above on value in other places.
$this->isUrl = true;
$imageContents = file_get_contents($path);
$this->isUrl = false;
if (preg_match('~^data:image/[a-z]+;base64,~', $path) === 1) {
$this->path = $path;

return $this;
}

$this->path = '';
// Check if a URL has been passed. https://stackoverflow.com/a/2058596/1252979
if (filter_var($path, FILTER_VALIDATE_URL)) {
if (!preg_match('/^(http|https|file|ftp|s3):/', $path)) {
throw new PhpSpreadsheetException('Invalid protocol for linked drawing');
}
// Implicit that it is a URL, rather store info than running check above on value in other places.
$this->isUrl = true;
$imageContents = @file_get_contents($path);
if ($imageContents !== false) {
$filePath = tempnam(sys_get_temp_dir(), 'Drawing');
if ($filePath) {
file_put_contents($filePath, $imageContents);
if (file_exists($filePath)) {
$this->setSizesAndType($filePath);
$put = @file_put_contents($filePath, $imageContents);
if ($put !== false) {
if ($this->isImage($filePath)) {
$this->path = $path;
$this->setSizesAndType($filePath);
}
unlink($filePath);
}
}
} elseif (file_exists($path)) {
$this->path = $path;
$this->setSizesAndType($path);
} elseif ($zip instanceof ZipArchive) {
$zipPath = explode('#', $path)[1];
if ($zip->locateName($zipPath) !== false) {
}
} elseif ($zip instanceof ZipArchive) {
$zipPath = explode('#', $path)[1];
$locate = @$zip->locateName($zipPath);
if ($locate !== false) {
if ($this->isImage($path)) {
$this->path = $path;
$this->setSizesAndType($path);
}
} else {
throw new PhpSpreadsheetException("File $path not found!");
}
} else {
$this->path = $path;
$exists = @file_exists($path);
if ($exists !== false && $this->isImage($path)) {
$this->path = $path;
$this->setSizesAndType($path);
}
}
if ($this->path === '' && $verifyFile) {
throw new PhpSpreadsheetException("File $path not found!");
}

return $this;
}

private function isImage(string $path): bool
{
$mime = (string) @mime_content_type($path);
$retVal = false;
if (str_starts_with($mime, 'image/')) {
$retVal = true;
} elseif ($mime === 'application/octet-stream') {
$extension = pathinfo($path, PATHINFO_EXTENSION);
$retVal = in_array($extension, ['bin', 'emf'], true);
}

return $retVal;
}

/**
* Get isURL.
*/
Expand Down
16 changes: 11 additions & 5 deletions src/PhpSpreadsheet/Writer/Html.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,9 @@ private function extendRowsForChartsAndImages(Worksheet $worksheet, int $row): s
[$rowMax, $colMax, $anyfound] = $this->extendRowsForCharts($worksheet, $row);

foreach ($worksheet->getDrawingCollection() as $drawing) {
if ($drawing instanceof Drawing && $drawing->getPath() === '') {
continue;
}
$anyfound = true;
$imageTL = Coordinate::coordinateFromString($drawing->getCoordinates());
$imageCol = Coordinate::columnIndexFromString($imageTL[0]);
Expand Down Expand Up @@ -687,7 +690,7 @@ private function writeImageInCell(Worksheet $worksheet, $coordinates)
}
$filedesc = $drawing->getDescription();
$filedesc = $filedesc ? htmlspecialchars($filedesc, ENT_QUOTES) : 'Embedded image';
if ($drawing instanceof Drawing) {
if ($drawing instanceof Drawing && $drawing->getPath() !== '') {
$filename = $drawing->getPath();

// Strip off eventual '.'
Expand All @@ -706,12 +709,15 @@ private function writeImageInCell(Worksheet $worksheet, $coordinates)
$imageData = self::winFileToUrl($filename, $this->isMPdf);

if ($this->embedImages || substr($imageData, 0, 6) === 'zip://') {
$imageData = 'data:,';
$picture = @file_get_contents($filename);
if ($picture !== false) {
$imageDetails = getimagesize($filename) ?: [];
// base64 encode the binary data
$base64 = base64_encode($picture);
$imageData = 'data:' . $imageDetails['mime'] . ';base64,' . $base64;
$mimeContentType = (string) @mime_content_type($filename);
if (substr($mimeContentType, 0, 6) === 'image/') {
// base64 encode the binary data
$base64 = base64_encode($picture);
$imageData = 'data:' . $mimeContentType . ';base64,' . $base64;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Writer/Xls.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ private function processDrawing(BstoreContainer &$bstoreContainer, Drawing $draw

private function processBaseDrawing(BstoreContainer &$bstoreContainer, BaseDrawing $drawing): void
{
if ($drawing instanceof Drawing) {
if ($drawing instanceof Drawing && $drawing->getPath() !== '') {
$this->processDrawing($bstoreContainer, $drawing);
} elseif ($drawing instanceof MemoryDrawing) {
$this->processMemoryDrawing($bstoreContainer, $drawing, $drawing->getRenderingFunction());
Expand Down
10 changes: 9 additions & 1 deletion src/PhpSpreadsheet/Writer/Xlsx.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,9 @@ public function save($filename, int $flags = 0): void

// Media
foreach ($this->spreadSheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
$zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath());
if ($image->getPath() !== '') {
$zipContent['xl/media/' . $image->getIndexedFilename()] = file_get_contents($image->getPath());
}
}
}

Expand All @@ -511,6 +513,9 @@ public function save($filename, int $flags = 0): void
if ($this->getDrawingHashTable()->getByIndex($i) instanceof WorksheetDrawing) {
$imageContents = null;
$imagePath = $this->getDrawingHashTable()->getByIndex($i)->getPath();
if ($imagePath === '') {
continue;
}
if (strpos($imagePath, 'zip://') !== false) {
$imagePath = substr($imagePath, 6);
$imagePathSplitted = explode('#', $imagePath);
Expand Down Expand Up @@ -712,6 +717,9 @@ private function processDrawing(WorksheetDrawing $drawing)
{
$data = null;
$filename = $drawing->getPath();
if ($filename === '') {
return null;
}
$imageData = getimagesize($filename);

if (!empty($imageData)) {
Expand Down
22 changes: 14 additions & 8 deletions src/PhpSpreadsheet/Writer/Xlsx/ContentTypes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use PhpOffice\PhpSpreadsheet\Shared\File;
use PhpOffice\PhpSpreadsheet\Shared\XMLWriter;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\Drawing as WorksheetDrawing;
use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing;
use PhpOffice\PhpSpreadsheet\Writer\Exception as WriterException;

Expand Down Expand Up @@ -132,18 +133,23 @@ public function writeContentTypes(Spreadsheet $spreadsheet, $includeCharts = fal
$extension = '';
$mimeType = '';

if ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof \PhpOffice\PhpSpreadsheet\Worksheet\Drawing) {
$extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getExtension());
$mimeType = $this->getImageMimeType($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getPath());
} elseif ($this->getParentWriter()->getDrawingHashTable()->getByIndex($i) instanceof MemoryDrawing) {
$extension = strtolower($this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType());
$drawing = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i);
if ($drawing instanceof WorksheetDrawing && $drawing->getPath() !== '') {
$extension = strtolower($drawing->getExtension());
if ($drawing->getIsUrl()) {
$mimeType = image_type_to_mime_type($drawing->getType());
} else {
$mimeType = $this->getImageMimeType($drawing->getPath());
}
} elseif ($drawing instanceof MemoryDrawing) {
$extension = strtolower($drawing->getMimeType());
$extension = explode('/', $extension);
$extension = $extension[1];

$mimeType = $this->getParentWriter()->getDrawingHashTable()->getByIndex($i)->getMimeType();
$mimeType = $drawing->getMimeType();
}

if (!isset($aMediaContentTypes[$extension])) {
if ($mimeType !== '' && !isset($aMediaContentTypes[$extension])) {
$aMediaContentTypes[$extension] = $mimeType;

$this->writeDefaultContentType($objWriter, $extension, $mimeType);
Expand All @@ -162,7 +168,7 @@ public function writeContentTypes(Spreadsheet $spreadsheet, $includeCharts = fal
for ($i = 0; $i < $sheetCount; ++$i) {
if (count($spreadsheet->getSheet($i)->getHeaderFooter()->getImages()) > 0) {
foreach ($spreadsheet->getSheet($i)->getHeaderFooter()->getImages() as $image) {
if (!isset($aMediaContentTypes[strtolower($image->getExtension())])) {
if ($image->getPath() !== '' && !isset($aMediaContentTypes[strtolower($image->getExtension())])) {
$aMediaContentTypes[strtolower($image->getExtension())] = $this->getImageMimeType($image->getPath());

$this->writeDefaultContentType($objWriter, strtolower($image->getExtension()), $aMediaContentTypes[strtolower($image->getExtension())]);
Expand Down
28 changes: 27 additions & 1 deletion tests/PhpSpreadsheetTests/Reader/Xlsx/URLImageTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@

namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;

use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
use PhpOffice\PhpSpreadsheet\IOFactory;
use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
use PhpOffice\PhpSpreadsheetTests\Reader\Utility\File;
use PHPUnit\Framework\TestCase;

class URLImageTest extends TestCase
{
public function testURLImageSource(): void
// https://github.com/readthedocs/readthedocs.org/issues/11615
public function xtestURLImageSource(): void
{
if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
self::markTestSkipped('Skipped due to setting of environment variable');
Expand Down Expand Up @@ -39,4 +41,28 @@ public function testURLImageSource(): void
self::assertSame('png', $extension);
}
}

public function xtestURLImageSourceNotFound(): void
{
if (getenv('SKIP_URL_IMAGE_TEST') === '1') {
self::markTestSkipped('Skipped due to setting of environment variable');
}
$filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.notfound.xlsx');
self::assertNotFalse($filename);
$reader = IOFactory::createReader('Xlsx');
$spreadsheet = $reader->load($filename);
$worksheet = $spreadsheet->getActiveSheet();
$collection = $worksheet->getDrawingCollection();
self::assertCount(0, $collection);
}

public function testURLImageSourceBadProtocol(): void
{
$filename = realpath(__DIR__ . '/../../../data/Reader/XLSX/urlImage.bad.dontuse');
self::assertNotFalse($filename);
$this->expectException(SpreadsheetException::class);
$this->expectExceptionMessage('Invalid protocol for linked drawing');
$reader = IOFactory::createReader('Xlsx');
$reader->load($filename);
}
}
Loading

0 comments on commit d95bc29

Please sign in to comment.