Skip to content

Commit 291d80a

Browse files
DeepDiver1975DeepDiver1975
authored
fix: perform SVG santitization on all file content which is pushed to… (#41433)
* fix: perform SVG santitization on all file content which is pushed to Imagick * fix: apply max file size limits to class Bitmap
1 parent 73f1419 commit 291d80a

File tree

11 files changed

+165
-20
lines changed

11 files changed

+165
-20
lines changed

‎changelog/unreleased/41433‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Bugfix: Apply SVG sanitization to all file content before using ImageMagick
2+
3+
Any file content is now sanitized for SVG threats before being processed by
4+
ImageMagick, preventing potential security vulnerabilities.
5+
6+
https://github.com/owncloud/core/pull/41433

‎lib/private/Image/ImagickFactory.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
namespace OC\Image;
4+
5+
use Imagick;
6+
use ImagickException;
7+
8+
class ImagickFactory {
9+
/**
10+
* @param mixed $files <p>
11+
* The path to an image to load or an array of paths. Paths can include
12+
* wildcards for file names, or can be URLs.
13+
* </p>
14+
* @return Imagick
15+
* @throws ImagickException
16+
*/
17+
public static function create($files = null): Imagick {
18+
$imagick = new Imagick($files);
19+
$imagick->setOption('svg:sanitize', 'true');
20+
$imagick->setOption('svg:embed', 'false');
21+
$imagick->setOption('svg:decode', 'true');
22+
return $imagick;
23+
}
24+
25+
}

‎lib/private/Preview.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use OC\Files\Filesystem;
3535
use OC\Files\View;
3636
use OCA\Files_Sharing\SharedMount;
37+
use OCP\Files\File;
3738
use OCP\Files\FileInfo;
3839
use OCP\Files\Folder;
3940
use OCP\Files\Node;
@@ -1081,6 +1082,22 @@ private function getPreviewPath($fileId = null) {
10811082
return $this->getThumbnailsFolder() . '/' . $fileId . '/';
10821083
}
10831084

1085+
/**
1086+
* @param File $file
1087+
* @return bool
1088+
* @throws \OCP\Files\InvalidPathException
1089+
* @throws \OCP\Files\NotFoundException
1090+
*/
1091+
public static function isImageFileSizeTooBig(File $file): bool {
1092+
$maxSizeForImages = \OC::$server->getConfig()->getSystemValue('preview_max_filesize_image', 50);
1093+
if ($maxSizeForImages === -1) {
1094+
return false;
1095+
}
1096+
$size = $file->getSize();
1097+
1098+
return $size > ($maxSizeForImages * 1024 * 1024);
1099+
}
1100+
10841101
/**
10851102
* Asks the provider to send a preview of the file which respects the maximum dimensions
10861103
* defined in the configuration and after saving it in the cache, it is then resized to the

‎lib/private/Preview/Bitmap.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
namespace OC\Preview;
2626

2727
use Imagick;
28+
use OC\Preview;
2829
use OCP\Files\File;
2930
use OCP\Files\FileInfo;
3031
use OCP\Preview\IProvider2;
@@ -40,6 +41,9 @@ abstract class Bitmap implements IProvider2 {
4041
* {@inheritDoc}
4142
*/
4243
public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
44+
if (Preview::isImageFileSizeTooBig($file)) {
45+
return false;
46+
}
4347
$stream = $file->fopen('r');
4448

4549
// Creates \Imagick object from bitmap or vector file
@@ -78,13 +82,21 @@ public function isAvailable(FileInfo $file) {
7882
* @param int $maxX
7983
* @param int $maxY
8084
*
81-
* @return \Imagick
85+
* @return Imagick
8286
*/
83-
private function getResizedPreview($stream, $maxX, $maxY) {
87+
private function getResizedPreview($stream, int $maxX, int $maxY): Imagick {
88+
# file content can be SVG - we need to sanitize it first
89+
$content = \stream_get_contents($stream);
90+
$output = SVG::sanitizeSVGContent($content);
91+
# in case the content is not an SVG we use the original content
92+
if ($output === '') {
93+
$output = $content;
94+
}
95+
8496
$bp = new Imagick();
8597

8698
# setIteratorIndex(0) will make previews to be generated from the first page
87-
$bp->readImageFile($stream);
99+
$bp->readImageBlob($output);
88100
$bp->setIteratorIndex(0);
89101

90102
$bp = $this->resize($bp, $maxX, $maxY);

‎lib/private/Preview/Image.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
*/
2727
namespace OC\Preview;
2828

29+
use OC\Preview;
2930
use OCP\Files\File;
3031
use OCP\Files\FileInfo;
3132
use OCP\Preview\IProvider2;
@@ -35,13 +36,9 @@ abstract class Image implements IProvider2 {
3536
* {@inheritDoc}
3637
*/
3738
public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
38-
$maxSizeForImages = \OC::$server->getConfig()->getSystemValue('preview_max_filesize_image', 50);
39-
$size = $file->getSize();
40-
41-
if ($maxSizeForImages !== -1 && $size > ($maxSizeForImages * 1024 * 1024)) {
39+
if (Preview::isImageFileSizeTooBig($file)) {
4240
return false;
4341
}
44-
4542
$image = new \OC_Image();
4643
$handle = $file->fopen('r');
4744
$image->load($handle);

‎lib/private/Preview/Office.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
namespace OC\Preview;
2626

27+
use OC\Image\ImagickFactory;
2728
use OCP\Files\File;
2829
use OCP\Files\FileInfo;
2930
use OCP\Preview\IProvider2;
@@ -60,22 +61,23 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
6061

6162
\shell_exec($exec);
6263

63-
//create imagick object from pdf
64+
//create imagick object from PDF
6465
$pdfPreview = null;
6566
try {
6667
$pathInfo = \pathinfo($absPath);
6768
$pdfPreview = $tmpDir . '/' . $pathInfo['filename'] . '.pdf';
6869

69-
$pdf = new \Imagick($pdfPreview . '[0]');
70-
$pdf->setImageFormat('jpg');
70+
# Note: no SVG sanitization of the file content required ....
71+
$imagick = ImagickFactory::create($pdfPreview . '[0]');
72+
$imagick->setImageFormat('jpg');
7173
} catch (\Exception $e) {
7274
@\unlink($pdfPreview);
7375
\OCP\Util::writeLog('core', $e->getmessage(), \OCP\Util::ERROR);
7476
return false;
7577
}
7678

7779
$image = new \OC_Image();
78-
$image->loadFromData($pdf);
80+
$image->loadFromData($imagick);
7981

8082
\unlink($pdfPreview);
8183

‎lib/private/Preview/SVG.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
*/
2424
namespace OC\Preview;
2525

26+
use OC\Image\ImagickFactory;
2627
use OCP\Files\File;
2728
use OCP\Files\FileInfo;
2829
use OCP\Preview\IProvider2;
@@ -41,8 +42,8 @@ public function getMimeType() {
4142
*/
4243
public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
4344
try {
44-
$svg = new \Imagick();
45-
$svg->setBackgroundColor(new \ImagickPixel('transparent'));
45+
$imagick = ImagickFactory::create();
46+
$imagick->setBackgroundColor(new \ImagickPixel('transparent'));
4647

4748
$stream = $file->fopen('r');
4849
$content = \stream_get_contents($stream);
@@ -54,16 +55,16 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
5455
# sanitize SVG content
5556
$output = self::sanitizeSVGContent($content);
5657

57-
$svg->readImageBlob($output);
58-
$svg->setImageFormat('png32');
58+
$imagick->readImageBlob($output);
59+
$imagick->setImageFormat('png32');
5960
} catch (\Exception $e) {
6061
\OCP\Util::writeLog('core', $e->getmessage(), \OCP\Util::ERROR);
6162
return false;
6263
}
6364

6465
//new image object
6566
$image = new \OC_Image();
66-
$image->loadFromData($svg);
67+
$image->loadFromData($imagick);
6768
//check if image object is valid
6869
if ($image->valid()) {
6970
$image->scaleDownToFit($maxX, $maxY);
@@ -84,6 +85,11 @@ public static function sanitizeSVGContent(string $content): string {
8485
$sanitizer = new DOMSanitizer(DOMSanitizer::SVG);
8586
$sanitizer->addDisallowedTags(['image']);
8687
$sanitizer->addDisallowedAttributes(['xlink:href']);
87-
return $sanitizer->sanitize($content);
88+
$sanitized_content = $sanitizer->sanitize($content);
89+
90+
// XML errors are expected here if the SVG is malformed
91+
\libxml_clear_errors();
92+
93+
return $sanitized_content;
8894
}
8995
}

‎tests/lib/Preview/SVGTest.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@
3030
*/
3131
class SVGTest extends Provider {
3232
public function setUp(): void {
33-
$checkImagick = new \Imagick();
34-
if (\count($checkImagick->queryFormats('SVG')) === 1) {
33+
if (\count(\Imagick::queryFormats('SVG')) === 1) {
3534
parent::setUp();
3635

3736
$fileName = 'testimagelarge.svg';

‎tests/lib/Preview/SanitizeTest.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
/**
3+
* @author Thomas Müller <thomas.mueller@tmit.eu>
4+
*
5+
* @copyright Copyright (c) 2026, ownCloud GmbH
6+
* @license AGPL-3.0
7+
*
8+
* This code is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Affero General Public License, version 3,
10+
* as published by the Free Software Foundation.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU Affero General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Affero General Public License, version 3,
18+
* along with this program. If not, see <http://www.gnu.org/licenses/>
19+
*
20+
*/
21+
22+
namespace Test\Preview;
23+
24+
use Generator;
25+
use OC\Preview\Bitmap;
26+
use OC\Preview\Font;
27+
use OC\Preview\PDF;
28+
use OCP\Files\File;
29+
use Test\TestCase;
30+
31+
class SanitizeTest extends TestCase {
32+
/**
33+
* @dataProvider providesSVG
34+
*/
35+
public function test(string $svgContent, Bitmap $provider): void {
36+
if (\count(\Imagick::queryFormats('SVG')) === 0) {
37+
$this->markTestSkipped('No SVG provider present');
38+
}
39+
# mock it all ....
40+
$stream = fopen('php://memory', 'rb+');
41+
fwrite($stream, $svgContent);
42+
rewind($stream);
43+
$file = $this->createMock(File::class);
44+
$file->method('getContent')->willReturn($svgContent);
45+
$file->method('fopen')->willReturn($stream);
46+
47+
# create the preview
48+
$return = $provider->getThumbnail($file, 32, 32, false);
49+
50+
$this->assertImage(__DIR__ . '/white-32x32.png', $return);
51+
}
52+
53+
public function providesSVG(): Generator {
54+
$embeddedImagePath = __DIR__ . '/../../data/testimage.jpg';
55+
$svgContent0 = <<<SVG
56+
<svg xmlns="http://www.w3.org/2000/svg" width="800" height="800">
57+
<image href="$embeddedImagePath" width="400" height="400"></image>
58+
</svg>
59+
SVG;
60+
61+
# all Bitmap based providers use the same thumbnailing logic - two is enough ....
62+
yield 'PDF provider' => [$svgContent0, new PDF()];
63+
yield 'Font Provider' => [$svgContent0, new Font()];
64+
}
65+
}

‎tests/lib/Preview/white-32x32.png‎

105 Bytes
Loading

0 commit comments

Comments
 (0)