Skip to content

Commit 27bbc54

Browse files
AltahrimAltahrim
committed
feat(previews): add file signature check before opening files
Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
1 parent d4aacd8 commit 27bbc54

11 files changed

Lines changed: 205 additions & 20 deletions

File tree

‎lib/private/Preview/Bitmap.php‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,25 @@
3838
*/
3939
abstract class Bitmap extends ProviderV2 {
4040

41+
/**
42+
* List of MIME types that this preview provider is allowed to process.
43+
*
44+
* These should correspond to the MIME types *identified* by Imagemagick
45+
* for files to be processed by this provider. These do / will not
46+
* necessarily need to match the MIME types stored in the database
47+
* (which are identified by IMimeTypeDetector).
48+
*
49+
* @return string Regular expression
50+
*/
51+
abstract protected function getAllowedMimeTypes();
52+
53+
/**
54+
* @return list<string>
55+
*/
56+
abstract protected function getMagicStrings();
57+
58+
abstract protected function getImagickFormatHint();
59+
4160
/**
4261
* {@inheritDoc}
4362
*/
@@ -78,12 +97,25 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
7897
* @param int $maxY
7998
*
8099
* @return \Imagick
100+
*
101+
* @throws \Exception
81102
*/
82103
private function getResizedPreview($tmpPath, $maxX, $maxY) {
83104
$bp = new Imagick();
84105

106+
if (!$this->isMagicStringSupported($tmpPath)) {
107+
throw new \Exception('Invalid image type: magic string not recognized');
108+
}
109+
110+
// Validate mime type
111+
$bp->pingImage($this->getImagickFormatHint() . ':' . $tmpPath . '[0]');
112+
$mimeType = $bp->getImageMimeType();
113+
if (!preg_match($this->getAllowedMimeTypes(), $mimeType)) {
114+
throw new \Exception('File mime type does not match the preview provider: ' . $mimeType);
115+
}
116+
85117
// Layer 0 contains either the bitmap or a flat representation of all vector layers
86-
$bp->readImage($tmpPath . '[0]');
118+
$bp->readImage($this->getImagickFormatHint() . ':' . $tmpPath . '[0]');
87119

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

@@ -92,6 +124,22 @@ private function getResizedPreview($tmpPath, $maxX, $maxY) {
92124
return $bp;
93125
}
94126

127+
private function isMagicStringSupported(string $filepath) {
128+
$signatures = $this->getMagicStrings();
129+
if (empty($signatures)) {
130+
return true;
131+
}
132+
$length = array_reduce($signatures, static fn (int $carry, string $signature) => max($carry, strlen($signature)), 0);
133+
$firstBytes = file_get_contents($filepath, false, null, 0, $length);
134+
foreach ($signatures as $signature) {
135+
if (str_starts_with($firstBytes, $signature)) {
136+
return true;
137+
}
138+
}
139+
140+
return false;
141+
}
142+
95143
/**
96144
* Returns a resized \Imagick object
97145
*

‎lib/private/Preview/Font.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,25 @@ class Font extends Bitmap {
2727
/**
2828
* {@inheritDoc}
2929
*/
30-
public function getMimeType(): string {
30+
public function getMimeType() {
3131
return '/application\/(?:font-sfnt|x-font$)/';
3232
}
33+
34+
/**
35+
* {@inheritDoc}
36+
*/
37+
protected function getAllowedMimeTypes() {
38+
return '/(application|image)\/(?:font-sfnt|x-font|x-otf|x-ttf|x-pfb$)/';
39+
}
40+
41+
protected function getMagicStrings() {
42+
return [
43+
"\x00\x01\x00\x00\x00", // TTF
44+
'OTTO', // OTF
45+
];
46+
}
47+
48+
protected function getImagickFormatHint() {
49+
return 'ttf';
50+
}
3351
}

‎lib/private/Preview/HEIC.php‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ class HEIC extends ProviderV2 {
4242
/**
4343
* {@inheritDoc}
4444
*/
45-
public function getMimeType(): string {
46-
return '/image\/hei(f|c)/';
45+
public function getMimeType() {
46+
return '/image\/(x-)?hei(f|c)/';
4747
}
4848

4949
/**
5050
* {@inheritDoc}
5151
*/
52-
public function isAvailable(\OCP\Files\FileInfo $file): bool {
52+
public function isAvailable(\OCP\Files\FileInfo $file) {
5353
return in_array('HEIC', \Imagick::queryFormats("HEI*"));
5454
}
5555

@@ -94,12 +94,22 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
9494
* @param int $maxY
9595
*
9696
* @return \Imagick
97+
*
98+
* @throws \Exception
9799
*/
98100
private function getResizedPreview($tmpPath, $maxX, $maxY) {
99101
$bp = new \Imagick();
100102

103+
// Some HEIC files just contain (or at least are identified as) other formats
104+
// like JPEG. We just need to check if the image is safe to process.
105+
$bp->pingImage('heic:' . $tmpPath . '[0]');
106+
$mimeType = $bp->getImageMimeType();
107+
if (!preg_match('/^image\/(x-)?(png|jpeg|gif|bmp|tiff|webp|hei(f|c)|avif)$/', $mimeType)) {
108+
throw new \Exception('File mime type does not match the preview provider: ' . $mimeType);
109+
}
110+
101111
// Layer 0 contains either the bitmap or a flat representation of all vector layers
102-
$bp->readImage($tmpPath . '[0]');
112+
$bp->readImage('heic:' . $tmpPath . '[0]');
103113

104114
$bp->setImageFormat('jpg');
105115

‎lib/private/Preview/Illustrator.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,22 @@ class Illustrator extends Bitmap {
2828
/**
2929
* {@inheritDoc}
3030
*/
31-
public function getMimeType(): string {
31+
public function getMimeType() {
3232
return '/application\/illustrator/';
3333
}
34+
35+
/**
36+
* {@inheritDoc}
37+
*/
38+
protected function getAllowedMimeTypes() {
39+
return '/application\/(illustrator|pdf)/';
40+
}
41+
42+
protected function getMagicStrings() {
43+
return ["\x25\x50\x44\x46"];
44+
}
45+
46+
protected function getImagickFormatHint() {
47+
return 'ai';
48+
}
3449
}

‎lib/private/Preview/PDF.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,22 @@ class PDF extends Bitmap {
2828
/**
2929
* {@inheritDoc}
3030
*/
31-
public function getMimeType(): string {
31+
public function getMimeType() {
3232
return '/application\/pdf/';
3333
}
34+
35+
/**
36+
* {@inheritDoc}
37+
*/
38+
protected function getAllowedMimeTypes() {
39+
return '/application\/pdf/';
40+
}
41+
42+
protected function getMagicStrings() {
43+
return ['%PDF-'];
44+
}
45+
46+
protected function getImagickFormatHint() {
47+
return 'pdf';
48+
}
3449
}

‎lib/private/Preview/Photoshop.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,22 @@ class Photoshop extends Bitmap {
2828
/**
2929
* {@inheritDoc}
3030
*/
31-
public function getMimeType(): string {
31+
public function getMimeType() {
3232
return '/application\/x-photoshop/';
3333
}
34+
35+
/**
36+
* {@inheritDoc}
37+
*/
38+
protected function getAllowedMimeTypes() {
39+
return '/(application|image)\/(x-photoshop|x-psd)/';
40+
}
41+
42+
protected function getMagicStrings() {
43+
return ['8BPS'];
44+
}
45+
46+
protected function getImagickFormatHint() {
47+
return 'psd';
48+
}
3449
}

‎lib/private/Preview/Postscript.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,22 @@ class Postscript extends Bitmap {
2828
/**
2929
* {@inheritDoc}
3030
*/
31-
public function getMimeType(): string {
31+
public function getMimeType() {
3232
return '/application\/postscript/';
3333
}
34+
35+
/**
36+
* {@inheritDoc}
37+
*/
38+
protected function getAllowedMimeTypes() {
39+
return '/application\/postscript/';
40+
}
41+
42+
protected function getMagicStrings() {
43+
return ['%!PS'];
44+
}
45+
46+
protected function getImagickFormatHint() {
47+
return 'ps';
48+
}
3449
}

‎lib/private/Preview/ProviderV2.php‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
use OCP\Files\File;
2929
use OCP\Files\FileInfo;
30-
use OCP\IImage;
3130
use OCP\Preview\IProviderV2;
3231

3332
abstract class ProviderV2 implements IProviderV2 {
@@ -47,15 +46,15 @@ public function __construct(array $options = []) {
4746
/**
4847
* @return string Regex with the mimetypes that are supported by this provider
4948
*/
50-
abstract public function getMimeType(): string ;
49+
abstract public function getMimeType();
5150

5251
/**
5352
* Check if a preview can be generated for $path
5453
*
5554
* @param FileInfo $file
5655
* @return bool
5756
*/
58-
public function isAvailable(FileInfo $file): bool {
57+
public function isAvailable(FileInfo $file) {
5958
return true;
6059
}
6160

@@ -68,7 +67,7 @@ public function isAvailable(FileInfo $file): bool {
6867
* @return null|\OCP\IImage false if no preview was generated
6968
* @since 17.0.0
7069
*/
71-
abstract public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage;
70+
abstract public function getThumbnail(File $file, int $maxX, int $maxY);
7271

7372
protected function useTempFile(File $file) {
7473
return $file->isEncrypted() || !$file->getStorage()->isLocal();
@@ -81,7 +80,7 @@ protected function useTempFile(File $file) {
8180
* @param int $maxSize maximum size for temporary files
8281
* @return string
8382
*/
84-
protected function getLocalFile(File $file, int $maxSize = null): string {
83+
protected function getLocalFile(File $file, int $maxSize = null) {
8584
if ($this->useTempFile($file)) {
8685
$absPath = \OC::$server->getTempManager()->getTemporaryFile();
8786

‎lib/private/Preview/SGI.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,22 @@ class SGI extends Bitmap {
2727
/**
2828
* {@inheritDoc}
2929
*/
30-
public function getMimeType(): string {
31-
return '/image\/sgi/';
30+
public function getMimeType() {
31+
return '/image\/(x-)?sgi/';
32+
}
33+
34+
/**
35+
* {@inheritDoc}
36+
*/
37+
protected function getAllowedMimeTypes() {
38+
return '/image\/(x-)?sgi/';
39+
}
40+
41+
protected function getMagicStrings() {
42+
return ["\x01\xDA"];
43+
}
44+
45+
protected function getImagickFormatHint() {
46+
return 'sgi';
3247
}
3348
}

‎lib/private/Preview/TGA.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,22 @@ class TGA extends Bitmap {
2727
/**
2828
* {@inheritDoc}
2929
*/
30-
public function getMimeType(): string {
31-
return '/image\/t(ar)?ga/';
30+
public function getMimeType() {
31+
return '/image\/(x-)?t(ar)?ga/';
32+
}
33+
34+
/**
35+
* {@inheritDoc}
36+
*/
37+
protected function getAllowedMimeTypes() {
38+
return '/image\/(x-)?t(ar)?ga/';
39+
}
40+
41+
protected function getMagicStrings() {
42+
return [];
43+
}
44+
45+
protected function getImagickFormatHint() {
46+
return 'tga';
3247
}
3348
}

0 commit comments

Comments
 (0)