CVE-2021-4116: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: yetiforcecrm is vulnerable to Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2021-12-15T14:15Z
Repository: https://github.com/yetiforcecompany/yetiforcecrm
Language: PHP
Files changed: app/Fields/File.php, config/version.php, languages/en-US/Other/Exceptions.json
Commit date: 2021-12-14 14:20:27 +0100
Commit message: Improved validation of uploaded files

Diff: diff --git a/app/Fields/File.php b/app/Fields/File.php
index d3fd25ac3b4..0cc183db215 100644
--- a/app/Fields/File.php
+++ b/app/Fields/File.php
@@ -123,10 +123,13 @@ class File
 	{
 		$instance = new self();
 		foreach ($fileInfo as $key => $value) {
 			$instance->{$key} = $fileInfo[$key];
 		}
+		if (isset($instance->name)) {
+			$instance->name = trim(\App\Purifier::purify($instance->name));
+		}
 		return $instance;
 	}
 
 	/**
 	 * Load file instance from request.
@@ -146,18 +149,18 @@ class File
 	}
 
 	/**
 	 * Load file instance from file path.
 	 *
-	 * @param array $path
+	 * @param string $path
 	 *
 	 * @return \self
 	 */
-	public static function loadFromPath($path)
+	public static function loadFromPath(string $path)
 	{
 		$instance = new self();
-		$instance->name = basename($path);
+		$instance->name = trim(\App\Purifier::purify(basename($path)));
 		$instance->path = $path;
 		return $instance;
 	}
 
 	/**
@@ -200,11 +203,11 @@ class File
 		}
 		if (mb_strlen($name) > 180) {
 			$name = \App\TextParser::textTruncate($name, 180, false) . '_' . uniqid() . ".$extension";
 		}
 		$instance = new self();
-		$instance->name = $name;
+		$instance->name = trim(\App\Purifier::purify($name));
 		$instance->path = $path;
 		$instance->ext = $extension;
 		foreach ($param as $key => $value) {
 			$instance->{$key} = $value;
 		}
@@ -375,17 +378,17 @@ class File
 	}
 
 	/**
 	 * Validate whether the file is safe.
 	 *
-	 * @param bool|string $type
+	 * @param string|null $type
 	 *
 	 * @throws \Exception
 	 *
 	 * @return bool
 	 */
-	public function validate($type = false)
+	public function validate(?string $type = null): bool
 	{
 		$return = true;
 		try {
 			if ($type && $this->getShortMimeType(0) !== $type) {
 				throw new \App\Exceptions\DangerousFile('ERR_FILE_ILLEGAL_FORMAT');
@@ -406,29 +409,29 @@ class File
 			} else {
 				$params = explode('||', $message);
 				$message = \call_user_func_array('vsprintf', [\App\Language::translateSingleMod(array_shift($params), 'Other.Exceptions'), $params]);
 			}
 			$this->validateError = $message;
-			Log::error("Error: {$e->getMessage()} | {$this->getName()} | {$this->getSize()}", __CLASS__);
+			Log::error("Error during file validation: {$this->getName()} | Size: {$this->getSize()}\n {$e->__toString()}", __CLASS__);
 		}
 		return $return;
 	}
 
 	/**
 	 * Validate and secure the file.
 	 *
-	 * @param bool|string $type
+	 * @param string|null $type
 	 *
 	 * @return bool
 	 */
-	public function validateAndSecure($type = false): bool
+	public function validateAndSecure(?string $type = null): bool
 	{
 		if ($this->validate($type)) {
 			return true;
 		}
 		$reValidate = false;
-		if ('image' === $this->getShortMimeType(0) && static::secureImage($this)) {
+		if (static::secureFile($this)) {
 			$this->size = filesize($this->path);
 			$this->content = file_get_contents($this->path);
 			$reValidate = true;
 		}
 		if ($reValidate && $this->validate($type)) {
@@ -478,10 +481,13 @@ class File
 			throw new \App\Exceptions\DangerousFile('ERR_FILE_ERROR_REQUEST||' . self::getErrorMessage($this->error));
 		}
 		if (empty($this->name)) {
 			throw new \App\Exceptions\DangerousFile('ERR_FILE_EMPTY_NAME');
 		}
+		if (!$this->validateInjection($this->name)) {
+			throw new \App\Exceptions\DangerousFile('ERR_FILE_ILLEGAL_NAME');
+		}
 		if (0 === $this->getSize()) {
 			throw new \App\Exceptions\DangerousFile('ERR_FILE_WRONG_SIZE');
 		}
 	}
 
@@ -531,11 +537,11 @@ class File
 			$contents = $this->getContents();
 			if ((1 === preg_match('/(<\?php?(.*?))/si', $contents)
 			|| false !== stripos($contents, '<?=')
 			|| false !== stripos($contents, '<? ')) && $this->searchCodeInjection()
 			) {
-				throw new \App\Exceptions\DangerousFile('ERR_FILE_PHP_CODE_INJECTION');
+				throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION');
 			}
 		}
 	}
 
 	/**
@@ -634,47 +640,61 @@ class File
 	 *
 	 * @throws \App\Exceptions\DangerousFile
 	 */
 	private function validateCodeInjectionInMetadata()
 	{
-		if (
+		if (\extension_loaded('imagick')) {
+			try {
+				$img = new \imagick($this->path);
+				$this->validateInjection($img->getImageProperties());
+			} catch (\Throwable $e) {
+				throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION', $e->getCode(), $e);
+			}
+		} elseif (
 			\function_exists('exif_read_data')
 			&& \in_array($this->getMimeType(), ['image/jpeg', 'image/tiff'])
 			&& \in_array(exif_imagetype($this->path), [IMAGETYPE_JPEG, IMAGETYPE_TIFF_II, IMAGETYPE_TIFF_MM])
 		) {
 			$imageSize = getimagesize($this->path, $imageInfo);
-			if (
-				$imageSize
-				&& (empty($imageInfo['APP1']) || 0 === strpos($imageInfo['APP1'], 'Exif'))
-				&& ($exifdata = exif_read_data($this->path)) && !$this->validateImageMetadata($exifdata)
-			) {
-				throw new \App\Exceptions\DangerousFile('ERR_FILE_PHP_CODE_INJECTION');
+			try {
+				if (
+					$imageSize
+					&& (empty($imageInfo['APP1']) || 0 === strpos($imageInfo['APP1'], 'Exif'))
+					&& ($exifData = exif_read_data($this->path)) && !$this->validateInjection($exifData)
+				) {
+					throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION');
+				}
+			} catch (\Throwable $e) {
+				throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION', $e->getCode(), $e);
 			}
 		}
 	}
 
 	/**
-	 * Validate image metadata.
+	 * Validate injection.
 	 *
-	 * @param mixed $data
+	 * @param string|array $data
 	 *
 	 * @return bool
 	 */
-	private function validateImageMetadata($data)
+	private function validateInjection($data): bool
 	{
+		$return = true;
 		if (\is_array($data)) {
 			foreach ($data as $value) {
-				if (!$this->validateImageMetadata($value)) {
+				if (!$this->validateInjection($value)) {
 					return false;
 				}
 			}
 		} else {
 			if (1 === preg_match('/(<\?php?(.*?))/i', $data) || false !== stripos($data, '<?=') || false !== stripos($data, '<? ')) {
-				return false;
+				$return = false;
+			} else {
+				\App\Purifier::purifyHtmlEventAttributes($data);
 			}
 		}
-		return true;
+		return $return;
 	}
 
 	/**
 	 * Get file ontent.
 	 *
@@ -1196,11 +1216,11 @@ class File
 		foreach (static::transform($files, true) as $key => $transformFiles) {
 			foreach ($transformFiles as $fileDetails) {
 				$additionalNotes = '';
 				$file = static::loadFromRequest($fileDetails);
 				if (!$file->validate($type)) {
-					if (!static::secureImage($file)) {
+					if (!static::secureFile($file)) {
 						$attach[] = ['name' => $file->getName(), 'error' => $file->validateError, 'hash' => $request->getByType('hash', 'Alnum')];
 						continue;
 					}
 					$fileDetails['size'] = filesize($fileDetails['tmp_name']);
 					$file = static::loadFromRequest($fileDetails);
@@ -1299,12 +1319,15 @@ class File
 	 *
 	 * @param \App\Fields\File $file
 	 *
 	 * @return bool
 	 */
-	public static function secureImage(self $file): bool
+	public static function secureFile(self $file): bool
 	{
+		if ('image' !== $file->getShortMimeType(0)) {
+			return false;
+		}
 		$result = false;
 		if (\extension_loaded('imagick')) {
 			try {
 				$img = new \imagick($file->getPath());
 				$img->stripImage();
diff --git a/config/version.php b/config/version.php
index 366143bbae2..34aa642feb1 100644
--- a/config/version.php
+++ b/config/version.php
@@ -1,7 +1,7 @@
 <?php
 
 return [
-	'appVersion' => '6.3.5',
+	'appVersion' => '6.3.6',
 	'patchVersion' => '2021.12.14',
 	'lib_roundcube' => '0.2.3',
 ];
diff --git a/languages/en-US/Other/Exceptions.json b/languages/en-US/Other/Exceptions.json
index 92e50d57222..ae68b0886ff 100644
--- a/languages/en-US/Other/Exceptions.json
+++ b/languages/en-US/Other/Exceptions.json
@@ -20,12 +20,13 @@
     "ERR_CREATE_FILE_FAILURE": "Failed to create file",
     "ERR_FILE_WRONG_SIZE": "Wrong file size",
     "ERR_FILE_EMPTY_NAME": "File name is empty",
     "ERR_FILE_ERROR_REQUEST": "File transfer error: %s",
     "ERR_FILE_ILLEGAL_FORMAT": "Illegal file format",
+    "ERR_FILE_ILLEGAL_NAME": "Illegal file name",
     "ERR_FILE_WRONG_IMAGE": "Wrong image file",
-    "ERR_FILE_PHP_CODE_INJECTION": "File contains dangerous PHP code and was rejected",
+    "ERR_FILE_CODE_INJECTION": "File contains dangerous code and was rejected",
     "ERR_FILE_XPACKET_CODE_INJECTION": "File contains dangerous XPACKET code and was rejected",
     "ERR_NO_PERMISSIONS_TO_CREATE_DIRECTORIES": "No permissions to create directories",
     "ERR_DIRECTORY_CANNOT_BE_CREATED": "The directory cannot be created",
     "ERR_AUTO_LOGOUT": "Logged out automatically due to no activity",
     "ERR_FIELD_NOT_FOUND": "Field not found (%s)",

Vulnerable code: 	 * @param array $path
	public static function loadFromPath($path)
		$instance->name = basename($path);
		$instance->name = $name;
	 * @param bool|string $type
	public function validate($type = false)
			Log::error("Error: {$e->getMessage()} | {$this->getName()} | {$this->getSize()}", __CLASS__);
	 * @param bool|string $type
	public function validateAndSecure($type = false): bool
		if ('image' === $this->getShortMimeType(0) && static::secureImage($this)) {
				throw new \App\Exceptions\DangerousFile('ERR_FILE_PHP_CODE_INJECTION');
		if (
			if (
				$imageSize
				&& (empty($imageInfo['APP1']) || 0 === strpos($imageInfo['APP1'], 'Exif'))
				&& ($exifdata = exif_read_data($this->path)) && !$this->validateImageMetadata($exifdata)
			) {
				throw new \App\Exceptions\DangerousFile('ERR_FILE_PHP_CODE_INJECTION');
	 * Validate image metadata.
	 * @param mixed $data
	private function validateImageMetadata($data)
				if (!$this->validateImageMetadata($value)) {
				return false;
		return true;
					if (!static::secureImage($file)) {
	public static function secureImage(self $file): bool
	'appVersion' => '6.3.5',
    "ERR_FILE_PHP_CODE_INJECTION": "File contains dangerous PHP code and was rejected",
Fixed code: 		if (isset($instance->name)) {
			$instance->name = trim(\App\Purifier::purify($instance->name));
		}
	 * @param string $path
	public static function loadFromPath(string $path)
		$instance->name = trim(\App\Purifier::purify(basename($path)));
		$instance->name = trim(\App\Purifier::purify($name));
	 * @param string|null $type
	public function validate(?string $type = null): bool
			Log::error("Error during file validation: {$this->getName()} | Size: {$this->getSize()}\n {$e->__toString()}", __CLASS__);
	 * @param string|null $type
	public function validateAndSecure(?string $type = null): bool
		if (static::secureFile($this)) {
		if (!$this->validateInjection($this->name)) {
			throw new \App\Exceptions\DangerousFile('ERR_FILE_ILLEGAL_NAME');
		}
				throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION');
		if (\extension_loaded('imagick')) {
			try {
				$img = new \imagick($this->path);
				$this->validateInjection($img->getImageProperties());
			} catch (\Throwable $e) {
				throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION', $e->getCode(), $e);
			}
		} elseif (
			try {
				if (
					$imageSize
					&& (empty($imageInfo['APP1']) || 0 === strpos($imageInfo['APP1'], 'Exif'))
					&& ($exifData = exif_read_data($this->path)) && !$this->validateInjection($exifData)
				) {
					throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION');
				}
			} catch (\Throwable $e) {
				throw new \App\Exceptions\DangerousFile('ERR_FILE_CODE_INJECTION', $e->getCode(), $e);
	 * Validate injection.
	 * @param string|array $data
	private function validateInjection($data): bool
		$return = true;
				if (!$this->validateInjection($value)) {
				$return = false;
			} else {
				\App\Purifier::purifyHtmlEventAttributes($data);
		return $return;
					if (!static::secureFile($file)) {
	public static function secureFile(self $file): bool
		if ('image' !== $file->getShortMimeType(0)) {
			return false;
		}
	'appVersion' => '6.3.6',
    "ERR_FILE_ILLEGAL_NAME": "Illegal file name",
    "ERR_FILE_CODE_INJECTION": "File contains dangerous code and was rejected",