From 3ad763cf18597a6d8bf0c5ead66eb6805f5f1ab8 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Thu, 16 Dec 2021 09:17:11 +0100 Subject: [PATCH] Prevent loading images that would require too much memory. For most image formats, the header specifies the width/height. PHP allocates an image object from that size, even if the actual image data is much smaller. This image object size is not limited by the limit configured in PHP. The memory limit can be configured through "config.php" setting "preview_max_memory" and defaults to 128 MBytes which should be enough for most images without filling up all memory. Signed-off-by: Joachim Bauch --- config/config.sample.php | 10 +++ lib/private/legacy/OC_Image.php | 103 ++++++++++++++++++++++++++++- tests/data/testimage-badheader.jpg | Bin 0 -> 103 bytes tests/lib/ImageTest.php | 17 +++++ 4 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 tests/data/testimage-badheader.jpg diff --git a/config/config.sample.php b/config/config.sample.php index 72429b62d590f..036bd69d34b70 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1023,6 +1023,16 @@ */ 'preview_max_filesize_image' => 50, +/** + * max memory for generating image previews with imagegd (default behavior) + * Reads the image dimensions from the header and assumes 32 bits per pixel. + * If creating the image would allocate more memory, preview generation will + * be disabled and the default mimetype icon is shown. Set to -1 for no limit. + * + * Defaults to ``128`` megabytes + */ +'preview_max_memory' => 128, + /** * custom path for LibreOffice/OpenOffice binary * diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index 9e754f57b7661..3750c1c97b0f2 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -47,6 +47,10 @@ * Class for basic image manipulation */ class OC_Image implements \OCP\IImage { + + // Default memory limit for images to load (128 MBytes). + protected const DEFAULT_MEMORY_LIMIT = 128; + /** @var false|resource|\GdImage */ protected $resource = false; // tmp resource. /** @var int */ @@ -564,6 +568,71 @@ public function loadFromFileHandle($handle) { return false; } + /** + * Check if allocating an image with the given size is allowed. + * + * @param int $width The image width. + * @param int $height The image height. + * @return bool true if allocating is allowed, false otherwise + */ + private function checkImageMemory($width, $height) { + $memory_limit = $this->config->getSystemValueInt('preview_max_memory', self::DEFAULT_MEMORY_LIMIT); + if ($memory_limit < 0) { + // Not limited. + return true; + } + + // Assume 32 bits per pixel. + if ($width * $height * 4 > $memory_limit * 1024 * 1024) { + $this->logger->debug('Image size of ' . $width . 'x' . $height . ' would exceed allowed memory limit of ' . $memory_limit); + return false; + } + + return true; + } + + /** + * Check if loading an image file from the given path is allowed. + * + * @param string $path The path to a local file. + * @return bool true if allocating is allowed, false otherwise + */ + private function checkImageSize($path) { + $size = getimagesize($path); + if (!$size) { + return true; + } + + $width = $size[0]; + $height = $size[1]; + if (!$this->checkImageMemory($width, $height)) { + return false; + } + + return true; + } + + /** + * Check if loading an image from the given data is allowed. + * + * @param string $data A string of image data as read from a file. + * @return bool true if allocating is allowed, false otherwise + */ + private function checkImageDataSize($data) { + $size = getimagesizefromstring($data); + if (!$size) { + return true; + } + + $width = $size[0]; + $height = $size[1]; + if (!$this->checkImageMemory($width, $height)) { + return false; + } + + return true; + } + /** * Loads an image from a local file. * @@ -579,6 +648,9 @@ public function loadFromFile($imagePath = false) { switch ($iType) { case IMAGETYPE_GIF: if (imagetypes() & IMG_GIF) { + if (!$this->checkImageSize($imagePath)) { + return false; + } $this->resource = imagecreatefromgif($imagePath); if ($this->resource) { // Preserve transparency @@ -593,6 +665,9 @@ public function loadFromFile($imagePath = false) { break; case IMAGETYPE_JPEG: if (imagetypes() & IMG_JPG) { + if (!$this->checkImageSize($imagePath)) { + return false; + } if (getimagesize($imagePath) !== false) { $this->resource = @imagecreatefromjpeg($imagePath); } else { @@ -604,6 +679,9 @@ public function loadFromFile($imagePath = false) { break; case IMAGETYPE_PNG: if (imagetypes() & IMG_PNG) { + if (!$this->checkImageSize($imagePath)) { + return false; + } $this->resource = @imagecreatefrompng($imagePath); if ($this->resource) { // Preserve transparency @@ -618,6 +696,9 @@ public function loadFromFile($imagePath = false) { break; case IMAGETYPE_XBM: if (imagetypes() & IMG_XPM) { + if (!$this->checkImageSize($imagePath)) { + return false; + } $this->resource = @imagecreatefromxbm($imagePath); } else { $this->logger->debug('OC_Image->loadFromFile, XBM/XPM images not supported: ' . $imagePath, ['app' => 'core']); @@ -625,6 +706,9 @@ public function loadFromFile($imagePath = false) { break; case IMAGETYPE_WBMP: if (imagetypes() & IMG_WBMP) { + if (!$this->checkImageSize($imagePath)) { + return false; + } $this->resource = @imagecreatefromwbmp($imagePath); } else { $this->logger->debug('OC_Image->loadFromFile, WBMP images not supported: ' . $imagePath, ['app' => 'core']); @@ -635,6 +719,9 @@ public function loadFromFile($imagePath = false) { break; case IMAGETYPE_WEBP: if (imagetypes() & IMG_WEBP) { + if (!$this->checkImageSize($imagePath)) { + return false; + } $this->resource = @imagecreatefromwebp($imagePath); } else { $this->logger->debug('OC_Image->loadFromFile, webp images not supported: ' . $imagePath, ['app' => 'core']); @@ -667,7 +754,11 @@ public function loadFromFile($imagePath = false) { default: // this is mostly file created from encrypted file - $this->resource = imagecreatefromstring(file_get_contents($imagePath)); + $data = file_get_contents($imagePath); + if (!$this->checkImageDataSize($data)) { + return false; + } + $this->resource = imagecreatefromstring($data); $iType = IMAGETYPE_PNG; $this->logger->debug('OC_Image->loadFromFile, Default', ['app' => 'core']); break; @@ -690,6 +781,9 @@ public function loadFromData($str) { if (!is_string($str)) { return false; } + if (!$this->checkImageDataSize($str)) { + return false; + } $this->resource = @imagecreatefromstring($str); if ($this->fileInfo) { $this->mimeType = $this->fileInfo->buffer($str); @@ -718,6 +812,9 @@ public function loadFromBase64($str) { } $data = base64_decode($str); if ($data) { // try to load from string data + if (!$this->checkImageDataSize($data)) { + return false; + } $this->resource = @imagecreatefromstring($data); if ($this->fileInfo) { $this->mimeType = $this->fileInfo->buffer($data); @@ -794,6 +891,10 @@ private function imagecreatefrombmp($fileName) { } } } + if (!$this->checkImageMemory($meta['width'], $meta['height'])) { + fclose($fh); + return false; + } // create gd image $im = imagecreatetruecolor($meta['width'], $meta['height']); if ($im == false) { diff --git a/tests/data/testimage-badheader.jpg b/tests/data/testimage-badheader.jpg new file mode 100644 index 0000000000000000000000000000000000000000..b876804eb4e22483f2cbb90862f06a15db87595d GIT binary patch literal 103 ycmex=method('getAppValue') ->with('preview', 'jpeg_quality', 90) ->willReturn(null); + $config->expects($this->once()) + ->method('getSystemValueInt') + ->with('preview_max_memory', 128) + ->willReturn(128); $img = new \OC_Image(null, null, $config); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.jpg'); $raw = imagecreatefromstring(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg')); @@ -363,4 +367,17 @@ public function testConvert($mimeType) { $img->save($tempFile, $mimeType); $this->assertEquals($mimeType, image_type_to_mime_type(exif_imagetype($tempFile))); } + + public function testMemoryLimitFromFile() { + $img = new \OC_Image(); + $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage-badheader.jpg'); + $this->assertFalse($img->valid()); + } + + public function testMemoryLimitFromData() { + $data = file_get_contents(OC::$SERVERROOT.'/tests/data/testimage-badheader.jpg'); + $img = new \OC_Image(); + $img->loadFromData($data); + $this->assertFalse($img->valid()); + } }