Merge branch 'security' into 'master'

#1790:Fixed - Security Issue:RCE in company logo with webservice api

See merge request vtiger/vtigercrm!906

modules/Settings/Vtiger/actions/CompanyDetailsSave.php

@@ -34,29 +34,14 @@ class Settings_Vtiger_CompanyDetailsSave_Action extends Settings_Vtiger_Basic_Ac
 			$saveLogo = $status = true;
 			$logoName = false;
 			if(!empty($_FILES['logo']['name'])) {
-				$logoDetails = $_FILES['logo'];
-				$fileType = explode('/', $logoDetails['type']);
-				$fileType = $fileType[1];
-
-				if (!$logoDetails['size'] || !in_array($fileType, Settings_Vtiger_CompanyDetails_Model::$logoSupportedFormats)) {
-					$saveLogo = false;
-				}
-
-				//mime type check
-				$mimeType = mime_content_type($logoDetails['tmp_name']);
-				$mimeTypeContents = explode('/', $mimeType);
-				if (!$logoDetails['size'] || $mimeTypeContents[0] != 'image' || !in_array($mimeTypeContents[1], Settings_Vtiger_CompanyDetails_Model::$logoSupportedFormats)) {
-					$saveLogo = false;
-				}
-
-				// Check for php code injection
-				$imageContents = file_get_contents($logoDetails["tmp_name"]);
-				if (preg_match('/(<\?php?(.*?))/i', $imageContents) == 1) {
-					$saveLogo = false;
-				}
-				if ($saveLogo) {
-					$logoName = ltrim(basename(' '.Vtiger_Util_Helper::sanitizeUploadFileName($logoDetails['name'], vglobal('upload_badext'))));
+                                $logoDetails = $_FILES['logo'];
+				$saveLogo = Vtiger_Functions::validateImage($logoDetails);
+                                global $upload_badext;// from config.inc.php
+				$binFileName = sanitizeUploadFileName($logoDetails['name'], $upload_badext);
+				if ($saveLogo && pathinfo($binFileName, PATHINFO_EXTENSION) != 'txt') {
 					$moduleModel->saveLogo($logoName);
+                                } else {
+					$saveLogo = false;
 				}
 			}else{
 				$saveLogo = true;
@@ -93,4 +78,4 @@ class Settings_Vtiger_CompanyDetailsSave_Action extends Settings_Vtiger_Basic_Ac
 	public function validateRequest(Vtiger_Request $request) {
 		$request->validateWriteAccess();
 	}
-}
\ No newline at end of file
+}

modules/Settings/Vtiger/actions/UpdateCompanyLogo.php

@@ -16,32 +16,21 @@ class Settings_Vtiger_UpdateCompanyLogo_Action extends Settings_Vtiger_Basic_Act
 		$moduleModel = Settings_Vtiger_CompanyDetails_Model::getInstance();
 
 		$saveLogo = $securityError = false;
-		$logoDetails = $_FILES['logo'];
-		$fileType = explode('/', $logoDetails['type']);
-		$fileType = $fileType[1];
-
-		$logoContent = file_get_contents($logoDetails['tmp_name']);
-		if (preg_match('(<\?php?(.*?))', $logoContent) != 0) {
-			$securityError = true;
-		}
-
-		if (!$securityError) {
-			if ($logoDetails['size'] && in_array($fileType, Settings_Vtiger_CompanyDetails_Model::$logoSupportedFormats)) {
-				$saveLogo = true;
-			}
-
-			if ($saveLogo) {
-				$logoName = ltrim(basename(' '.Vtiger_Util_Helper::sanitizeUploadFileName($logoDetails['name'], vglobal('upload_badext'))));
-				$moduleModel->saveLogo();
-				$moduleModel->set('logoname', $logoName);
-				$moduleModel->save();
-			}
-		}
+                $logoDetails = $_FILES['logo'];
+                $saveLogo = Vtiger_Functions::validateImage($logoDetails);
+                if ($saveLogo) {
+                        $sanitizedFileName = ltrim(basename(' '.Vtiger_Util_Helper::sanitizeUploadFileName($logoDetails['name'], vglobal('upload_badext'))));
+                        if(pathinfo($sanitizedFileName, PATHINFO_EXTENSION) != 'txt'){
+                                $moduleModel->saveLogo($sanitizedFileName);
+                                $moduleModel->set('logoname', $sanitizedFileName);
+                                $moduleModel->save();
+                        }else {
+                                $saveLogo = false;
+                        }
+                }
 
 		$reloadUrl = $moduleModel->getIndexViewUrl();
-		if ($securityError) {
-			$reloadUrl .= '&error=LBL_IMAGE_CORRUPTED';
-		} else if (!$saveLogo) {
+		if (!$saveLogo) {
 			$reloadUrl .= '&error=LBL_INVALID_IMAGE';
 		}
 		header('Location: ' . $reloadUrl);

test/logo/.htaccess

+RewriteEngine on
+<FilesMatch ".*">
+    Order deny,allow
+    Deny from all
+</FilesMatch>
+
+<FilesMatch "\.(gif|jpe?g|png|bmp|PNG|GIF|BMP|jpg|JPG|ico)$">
+   allow from all
+</FilesMatch>