From c3aa27cc79ee65547278a367a6343e9a76a6769a Mon Sep 17 00:00:00 2001
From: MGatner <mgatner@icloud.com>
Date: Tue, 18 Feb 2020 15:13:08 -0500
Subject: [PATCH 1/4] Add image file verification and tests

---
 system/Images/Exceptions/ImageException.php   | 10 +++
 system/Images/Handlers/BaseHandler.php        | 83 ++++++++++++++-----
 system/Images/Handlers/GDHandler.php          | 30 +++----
 system/Images/Handlers/ImageMagickHandler.php | 16 ++--
 tests/system/Images/BaseHandlerTest.php       | 18 ++++
 5 files changed, 115 insertions(+), 42 deletions(-)

diff --git a/system/Images/Exceptions/ImageException.php b/system/Images/Exceptions/ImageException.php
index 10218248cad7..a438f3b6bf88 100644
--- a/system/Images/Exceptions/ImageException.php
+++ b/system/Images/Exceptions/ImageException.php
@@ -5,6 +5,11 @@
 
 class ImageException extends FrameworkException implements ExceptionInterface
 {
+	public static function forMissingImage()
+	{
+		return new static(lang('Images.sourceImageRequired'));
+	}
+
 	public static function forMissingAngle()
 	{
 		return new static(lang('Images.rotationAngleRequired'));
@@ -15,6 +20,11 @@ public static function forInvalidDirection(string $dir = null)
 		return new static(lang('Images.invalidDirection', [$dir]));
 	}
 
+	public static function forInvalidPath()
+	{
+		return new static(lang('Images.invalidPath'));
+	}
+
 	public static function forEXIFUnsupported()
 	{
 		return new static(lang('Images.exifNotSupported'));
diff --git a/system/Images/Handlers/BaseHandler.php b/system/Images/Handlers/BaseHandler.php
index 3bd61e8bdb3d..5bec8d8c8468 100644
--- a/system/Images/Handlers/BaseHandler.php
+++ b/system/Images/Handlers/BaseHandler.php
@@ -62,6 +62,13 @@ abstract class BaseHandler implements ImageHandlerInterface
 	 */
 	protected $image = null;
 
+	/**
+	 * Whether the image file has been confirmed.
+	 *
+	 * @var bool
+	 */
+	protected $verified = false;
+
 	/**
 	 * Image width.
 	 *
@@ -158,6 +165,7 @@ public function withFile(string $path)
 		// Clear out the old resource so that
 		// it doesn't try to use a previous image
 		$this->resource = null;
+		$this->checked  = false;
 
 		$this->image = new Image($path, true);
 
@@ -177,9 +185,9 @@ protected function ensureResource()
 	{
 		if ($this->resource === null)
 		{
-			$path = $this->image->getPathname();
+			$path = $this->image()->getPathname();
 			// if valid image type, make corresponding image resource
-			switch ($this->image->imageType)
+			switch ($this->image()->imageType)
 			{
 				case IMAGETYPE_GIF:
 					$this->resource = imagecreatefromgif($path);
@@ -206,6 +214,43 @@ public function getFile()
 		return $this->image;
 	}
 
+	/**
+	 * Verifies that a file has been supplied and it is an image.
+	 *
+	 * @return Image  The image instance
+	 * @throws type ImageException
+	 */
+	protected function image(): ?Image
+	{
+		if ($this->checked)
+		{
+			return $this->image;
+		}
+
+		// Verify withFile has been called
+		if (empty($this->image))
+		{
+			throw ImageException::forMissingImage();
+		}
+
+		// Verify the loaded image is an Image instance
+		if (! $this->image instanceof Image)
+		{
+			throw ImageException::forInvalidPath();
+		}
+
+		// File::__construct has verified the file exists - make sure it is an image
+		if (! is_int($this->image->imageType))
+		{
+			throw ImageException::forInvalidPath();
+		}
+
+		// Note that the image has been verified
+		$this->checked = true;
+
+		return $this->image;
+	}
+
 	//--------------------------------------------------------------------
 
 	/**
@@ -236,7 +281,7 @@ public function getResource()
 	public function resize(int $width, int $height, bool $maintainRatio = false, string $masterDim = 'auto')
 	{
 		// If the target width/height match the source, then we have nothing to do here.
-		if ($this->image->origWidth === $width && $this->image->origHeight === $height)
+		if ($this->image()->origWidth === $width && $this->image()->origHeight === $height)
 		{
 			return $this;
 		}
@@ -302,7 +347,7 @@ public function crop(int $width = null, int $height = null, int $x = null, int $
 	 */
 	public function convert(int $imageType)
 	{
-		$this->image->imageType = $imageType;
+		$this->image()->imageType = $imageType;
 		return $this;
 	}
 
@@ -359,8 +404,8 @@ public function rotate(float $angle)
 	 */
 	public function flatten(int $red = 255, int $green = 255, int $blue = 255)
 	{
-		$this->width  = $this->image->origWidth;
-		$this->height = $this->image->origHeight;
+		$this->width  = $this->image()->origWidth;
+		$this->height = $this->image()->origHeight;
 
 		return $this->_flatten();
 	}
@@ -538,11 +583,11 @@ public function getEXIF(string $key = null, bool $silent = false)
 		}
 
 		$exif = null; // default
-		switch ($this->image->imageType)
+		switch ($this->image()->imageType)
 		{
 			case IMAGETYPE_JPEG:
 			case IMAGETYPE_TIFF_II:
-				$exif = exif_read_data($this->image->getPathname());
+				$exif = exif_read_data($this->image()->getPathname());
 				if (! is_null($key) && is_array($exif))
 				{
 					$exif = $exif[$key] ?? false;
@@ -576,8 +621,8 @@ public function getEXIF(string $key = null, bool $silent = false)
 	 */
 	public function fit(int $width, int $height = null, string $position = 'center')
 	{
-		$origWidth  = $this->image->origWidth;
-		$origHeight = $this->image->origHeight;
+		$origWidth  = $this->image()->origWidth;
+		$origHeight = $this->image()->origHeight;
 
 		list($cropWidth, $cropHeight) = $this->calcAspectRatio($width, $height, $origWidth, $origHeight);
 
@@ -749,9 +794,9 @@ protected abstract function process(string $action);
 	 */
 	public function __call(string $name, array $args = [])
 	{
-		if (method_exists($this->image, $name))
+		if (method_exists($this->image(), $name))
 		{
-			return $this->image->$name(...$args);
+			return $this->image()->$name(...$args);
 		}
 	}
 
@@ -772,11 +817,11 @@ public function __call(string $name, array $args = [])
 	protected function reproportion()
 	{
 		if (($this->width === 0 && $this->height === 0) ||
-				$this->image->origWidth === 0 ||
-				$this->image->origHeight === 0 ||
+				$this->image()->origWidth === 0 ||
+				$this->image()->origHeight === 0 ||
 				( ! ctype_digit((string) $this->width) && ! ctype_digit((string) $this->height)) ||
-				! ctype_digit((string) $this->image->origWidth) ||
-				! ctype_digit((string) $this->image->origHeight)
+				! ctype_digit((string) $this->image()->origWidth) ||
+				! ctype_digit((string) $this->image()->origHeight)
 		)
 		{
 			return;
@@ -790,7 +835,7 @@ protected function reproportion()
 		{
 			if ($this->width > 0 && $this->height > 0)
 			{
-				$this->masterDim = ((($this->image->origHeight / $this->image->origWidth) - ($this->height / $this->width)) < 0) ? 'width' : 'height';
+				$this->masterDim = ((($this->image()->origHeight / $this->image()->origWidth) - ($this->height / $this->width)) < 0) ? 'width' : 'height';
 			}
 			else
 			{
@@ -805,11 +850,11 @@ protected function reproportion()
 
 		if ($this->masterDim === 'width')
 		{
-			$this->height = (int) ceil($this->width * $this->image->origHeight / $this->image->origWidth);
+			$this->height = (int) ceil($this->width * $this->image()->origHeight / $this->image()->origWidth);
 		}
 		else
 		{
-			$this->width = (int) ceil($this->image->origWidth * $this->height / $this->image->origHeight);
+			$this->width = (int) ceil($this->image()->origWidth * $this->height / $this->image()->origHeight);
 		}
 	}
 
diff --git a/system/Images/Handlers/GDHandler.php b/system/Images/Handlers/GDHandler.php
index 7f24af5d9c71..0ef75aac71af 100644
--- a/system/Images/Handlers/GDHandler.php
+++ b/system/Images/Handlers/GDHandler.php
@@ -151,8 +151,8 @@ public function _flip(string $direction)
 	{
 		$srcImg = $this->createImage();
 
-		$width  = $this->image->origWidth;
-		$height = $this->image->origHeight;
+		$width  = $this->image()->origWidth;
+		$height = $this->image()->origHeight;
 
 		if ($direction === 'horizontal')
 		{
@@ -254,8 +254,8 @@ public function _crop()
 	 */
 	protected function process(string $action)
 	{
-		$origWidth  = $this->image->origWidth;
-		$origHeight = $this->image->origHeight;
+		$origWidth  = $this->image()->origWidth;
+		$origHeight = $this->image()->origHeight;
 
 		if ($action === 'crop')
 		{
@@ -266,8 +266,8 @@ protected function process(string $action)
 			// Modify the "original" width/height to the new
 			// values so that methods that come after have the
 			// correct size to work with.
-			$this->image->origHeight = $this->height;
-			$this->image->origWidth  = $this->width;
+			$this->image()->origHeight = $this->height;
+			$this->image()->origWidth  = $this->width;
 		}
 
 		// Create the image handle
@@ -286,7 +286,7 @@ protected function process(string $action)
 
 		$dest = $create($this->width, $this->height);
 
-		if ($this->image->imageType === IMAGETYPE_PNG) // png we can actually preserve transparency
+		if ($this->image()->imageType === IMAGETYPE_PNG) // png we can actually preserve transparency
 		{
 			imagealphablending($dest, false);
 			imagesavealpha($dest, true);
@@ -318,9 +318,9 @@ protected function process(string $action)
 	 */
 	public function save(string $target = null, int $quality = 90): bool
 	{
-		$target = empty($target) ? $this->image->getPathname() : $target;
+		$target = empty($target) ? $this->image()->getPathname() : $target;
 
-		switch ($this->image->imageType)
+		switch ($this->image()->imageType)
 		{
 			case IMAGETYPE_GIF:
 				if (! function_exists('imagegif'))
@@ -389,12 +389,12 @@ protected function createImage(string $path = '', string $imageType = '')
 
 		if ($path === '')
 		{
-			$path = $this->image->getPathname();
+			$path = $this->image()->getPathname();
 		}
 
 		if ($imageType === '')
 		{
-			$imageType = $this->image->imageType;
+			$imageType = $this->image()->imageType;
 		}
 
 		switch ($imageType)
@@ -490,21 +490,21 @@ protected function _text(string $text, array $options = [])
 		if ($options['vAlign'] === 'middle')
 		{
 			// Don't apply padding when you're in the middle of the image.
-			$yAxis += ($this->image->origHeight / 2) + ($fontheight / 2) - $options['padding'];
+			$yAxis += ($this->image()->origHeight / 2) + ($fontheight / 2) - $options['padding'];
 		}
 		elseif ($options['vAlign'] === 'bottom')
 		{
-			$yAxis = ($this->image->origHeight - $fontheight - $options['shadowOffset'] - ($fontheight / 2)) - $yAxis;
+			$yAxis = ($this->image()->origHeight - $fontheight - $options['shadowOffset'] - ($fontheight / 2)) - $yAxis;
 		}
 
 		// Set horizontal alignment
 		if ($options['hAlign'] === 'right')
 		{
-			$xAxis += ($this->image->origWidth - ($fontwidth * strlen($text)) - $options['shadowOffset']) - (2 * $options['padding']);
+			$xAxis += ($this->image()->origWidth - ($fontwidth * strlen($text)) - $options['shadowOffset']) - (2 * $options['padding']);
 		}
 		elseif ($options['hAlign'] === 'center')
 		{
-			$xAxis += floor(($this->image->origWidth - ($fontwidth * strlen($text))) / 2);
+			$xAxis += floor(($this->image()->origWidth - ($fontwidth * strlen($text))) / 2);
 		}
 
 		$options['xAxis'] = $xAxis;
diff --git a/system/Images/Handlers/ImageMagickHandler.php b/system/Images/Handlers/ImageMagickHandler.php
index d786166a593e..cc5f577dc234 100644
--- a/system/Images/Handlers/ImageMagickHandler.php
+++ b/system/Images/Handlers/ImageMagickHandler.php
@@ -97,7 +97,7 @@ public function __construct($config = null)
 	 */
 	public function _resize(bool $maintainRatio = false)
 	{
-		$source      = ! empty($this->resource) ? $this->resource : $this->image->getPathname();
+		$source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
 		$destination = $this->getResourcePath();
 
 		$escape = '\\';
@@ -123,7 +123,7 @@ public function _resize(bool $maintainRatio = false)
 	 */
 	public function _crop()
 	{
-		$source      = ! empty($this->resource) ? $this->resource : $this->image->getPathname();
+		$source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
 		$destination = $this->getResourcePath();
 
 		$action = ' -crop ' . $this->width . 'x' . $this->height . '+' . $this->xAxis . '+' . $this->yAxis . ' "' . $source . '" "' . $destination . '"';
@@ -148,7 +148,7 @@ protected function _rotate(int $angle)
 	{
 		$angle = '-rotate ' . $angle;
 
-		$source      = ! empty($this->resource) ? $this->resource : $this->image->getPathname();
+		$source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
 		$destination = $this->getResourcePath();
 
 		$action = ' ' . $angle . ' "' . $source . '" "' . $destination . '"';
@@ -174,7 +174,7 @@ public function _flatten(int $red = 255, int $green = 255, int $blue = 255)
 	{
 		$flatten = "-background RGB({$red},{$green},{$blue}) -flatten";
 
-		$source      = ! empty($this->resource) ? $this->resource : $this->image->getPathname();
+		$source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
 		$destination = $this->getResourcePath();
 
 		$action = ' ' . $flatten . ' "' . $source . '" "' . $destination . '"';
@@ -198,7 +198,7 @@ public function _flip(string $direction)
 	{
 		$angle = $direction === 'horizontal' ? '-flop' : '-flip';
 
-		$source      = ! empty($this->resource) ? $this->resource : $this->image->getPathname();
+		$source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
 		$destination = $this->getResourcePath();
 
 		$action = ' ' . $angle . ' "' . $source . '" "' . $destination . '"';
@@ -286,7 +286,7 @@ protected function process(string $action, int $quality = 100): array
 	 */
 	public function save(string $target = null, int $quality = 90): bool
 	{
-		$target = empty($target) ? $this->image : $target;
+		$target = empty($target) ? $this->image() : $target;
 
 		// If no new resource has been created, then we're
 		// simply copy the existing one.
@@ -295,7 +295,7 @@ public function save(string $target = null, int $quality = 90): bool
 			$name = basename($target);
 			$path = pathinfo($target, PATHINFO_DIRNAME);
 
-			return $this->image->copy($path, $name);
+			return $this->image()->copy($path, $name);
 		}
 
 		// Copy the file through ImageMagick so that it has
@@ -433,7 +433,7 @@ protected function _text(string $text, array $options = [])
 		// Text
 		$cmd .= " -annotate 0 '{$text}'";
 
-		$source      = ! empty($this->resource) ? $this->resource : $this->image->getPathname();
+		$source      = ! empty($this->resource) ? $this->resource : $this->image()->getPathname();
 		$destination = $this->getResourcePath();
 
 		$cmd = " '{$source}' {$cmd} '{$destination}'";
diff --git a/tests/system/Images/BaseHandlerTest.php b/tests/system/Images/BaseHandlerTest.php
index c0353c261674..33f7f77fa11b 100644
--- a/tests/system/Images/BaseHandlerTest.php
+++ b/tests/system/Images/BaseHandlerTest.php
@@ -68,6 +68,24 @@ public function testMissingFile()
 		$handler->withFile($this->start . 'No_such_file.jpg');
 	}
 
+	public function testNonImageFile()
+	{
+		$this->expectException(\CodeIgniter\Images\Exceptions\ImageException::class);
+		$handler = Services::image('gd', null, false);
+		$handler->withFile(SUPPORTPATH . 'Files/baker/banana.php');
+
+		// Make any call that accesses the image
+		$handler->resize(100, 100);
+	}
+
+	public function testForgotWithFile()
+	{
+		$this->expectException(\CodeIgniter\Images\Exceptions\ImageException::class);
+
+		// Make any call that accesses the image
+		$handler->resize(100, 100);
+	}
+
 	public function testFileTypes()
 	{
 		$handler = Services::image('gd', null, false);

From eadcfe60000a030c5480d25c3987ed228ac059ab Mon Sep 17 00:00:00 2001
From: MGatner <mgatner@icloud.com>
Date: Tue, 18 Feb 2020 16:12:12 -0500
Subject: [PATCH 2/4] Bugfix missing handler

---
 tests/system/Images/BaseHandlerTest.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/system/Images/BaseHandlerTest.php b/tests/system/Images/BaseHandlerTest.php
index 33f7f77fa11b..ecd42e69885c 100644
--- a/tests/system/Images/BaseHandlerTest.php
+++ b/tests/system/Images/BaseHandlerTest.php
@@ -81,6 +81,7 @@ public function testNonImageFile()
 	public function testForgotWithFile()
 	{
 		$this->expectException(\CodeIgniter\Images\Exceptions\ImageException::class);
+		$handler = Services::image('gd', null, false);
 
 		// Make any call that accesses the image
 		$handler->resize(100, 100);

From 64fff63f8ac1d5a26653bc779f55a90b7369a72e Mon Sep 17 00:00:00 2001
From: MGatner <mgatner@icloud.com>
Date: Tue, 18 Feb 2020 21:01:01 -0500
Subject: [PATCH 3/4] Bugfix inconsistent verify property name

---
 system/Images/Handlers/BaseHandler.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/system/Images/Handlers/BaseHandler.php b/system/Images/Handlers/BaseHandler.php
index 5bec8d8c8468..05e155f65ab5 100644
--- a/system/Images/Handlers/BaseHandler.php
+++ b/system/Images/Handlers/BaseHandler.php
@@ -165,7 +165,7 @@ public function withFile(string $path)
 		// Clear out the old resource so that
 		// it doesn't try to use a previous image
 		$this->resource = null;
-		$this->checked  = false;
+		$this->verified = false;
 
 		$this->image = new Image($path, true);
 
@@ -222,7 +222,7 @@ public function getFile()
 	 */
 	protected function image(): ?Image
 	{
-		if ($this->checked)
+		if ($this->verified)
 		{
 			return $this->image;
 		}
@@ -246,7 +246,7 @@ protected function image(): ?Image
 		}
 
 		// Note that the image has been verified
-		$this->checked = true;
+		$this->verified = true;
 
 		return $this->image;
 	}

From 7f347e0b0429376a01cd7bae0225377378e50c19 Mon Sep 17 00:00:00 2001
From: MGatner <mgatner@icloud.com>
Date: Wed, 19 Feb 2020 10:31:10 -0500
Subject: [PATCH 4/4] Add exception for unsupported files; move check to Image

---
 system/Images/Exceptions/ImageException.php | 5 +++++
 system/Images/Handlers/BaseHandler.php      | 2 +-
 system/Images/Image.php                     | 6 +++++-
 system/Language/en/Images.php               | 1 +
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/system/Images/Exceptions/ImageException.php b/system/Images/Exceptions/ImageException.php
index a438f3b6bf88..c514c7c02796 100644
--- a/system/Images/Exceptions/ImageException.php
+++ b/system/Images/Exceptions/ImageException.php
@@ -10,6 +10,11 @@ public static function forMissingImage()
 		return new static(lang('Images.sourceImageRequired'));
 	}
 
+	public static function forFileNotSupported()
+	{
+		return new static(lang('Images.fileNotSupported'));
+	}
+
 	public static function forMissingAngle()
 	{
 		return new static(lang('Images.rotationAngleRequired'));
diff --git a/system/Images/Handlers/BaseHandler.php b/system/Images/Handlers/BaseHandler.php
index 05e155f65ab5..990ad23ca996 100644
--- a/system/Images/Handlers/BaseHandler.php
+++ b/system/Images/Handlers/BaseHandler.php
@@ -242,7 +242,7 @@ protected function image(): ?Image
 		// File::__construct has verified the file exists - make sure it is an image
 		if (! is_int($this->image->imageType))
 		{
-			throw ImageException::forInvalidPath();
+			throw ImageException::forFileNotSupported();
 		}
 
 		// Note that the image has been verified
diff --git a/system/Images/Image.php b/system/Images/Image.php
index 904c78b36ab4..f67e8b35c65d 100644
--- a/system/Images/Image.php
+++ b/system/Images/Image.php
@@ -137,7 +137,11 @@ public function getProperties(bool $return = false)
 	{
 		$path = $this->getPathname();
 
-		$vals  = getimagesize($path);
+		if (! $vals = getimagesize($path))
+		{
+			throw ImageException::forFileNotSupported();
+		}
+
 		$types = [
 			1 => 'gif',
 			2 => 'jpeg',
diff --git a/system/Language/en/Images.php b/system/Language/en/Images.php
index 9d11da59a600..3dd8c0ee0858 100644
--- a/system/Language/en/Images.php
+++ b/system/Language/en/Images.php
@@ -21,6 +21,7 @@
    'gifNotSupported'        => 'GIF images are often not supported due to licensing restrictions. You may have to use JPG or PNG images instead.',
    'jpgNotSupported'        => 'JPG images are not supported.',
    'pngNotSupported'        => 'PNG images are not supported.',
+   'fileNotSupported'       => 'The supplied file is not a supported image type.',
    'unsupportedImageCreate' => 'Your server does not support the GD function required to process this type of image.',
    'jpgOrPngRequired'       => 'The image resize protocol specified in your preferences only works with JPEG or PNG image types.',
    'rotateUnsupported'      => 'Image rotation does not appear to be supported by your server.',