From 88b4c2d826cfbe37d306b08dc8692937955fa897 Mon Sep 17 00:00:00 2001 From: Uma <uma.s@vtiger.com> Date: Tue, 3 Sep 2019 17:07:18 +0530 Subject: [PATCH] File security through obscurity is supported --- data/CRMEntity.php | 7 ++++--- .../modules/Accounts/DetailViewHeaderTitle.tpl | 4 ++-- .../modules/Invoice/DetailViewHeaderTitle.tpl | 4 ++-- .../v7/modules/Leads/DetailViewHeaderTitle.tpl | 4 ++-- .../PurchaseOrder/DetailViewHeaderTitle.tpl | 4 ++-- .../v7/modules/Quotes/DetailViewHeaderTitle.tpl | 4 ++-- .../SalesOrder/DetailViewHeaderTitle.tpl | 4 ++-- layouts/v7/modules/Users/UserViewHeader.tpl | 2 +- layouts/v7/modules/Vtiger/EmailPreview.tpl | 4 ++-- modules/Documents/models/Record.php | 8 +++++--- modules/Emails/actions/DownloadFile.php | 8 +++++--- modules/Migration/schema/711_to_720.php | 17 +++++++++++++++++ modules/Users/Users.php | 7 ++++--- modules/Vtiger/helpers/ShowFile.php | 9 +++++---- modules/Vtiger/helpers/Util.php | 2 +- modules/Vtiger/models/Record.php | 5 +++-- vtlib/Vtiger/Functions.php | 2 +- 17 files changed, 60 insertions(+), 35 deletions(-) create mode 100644 modules/Migration/schema/711_to_720.php diff --git a/data/CRMEntity.php b/data/CRMEntity.php index 64650430e..5ad321e65 100644 --- a/data/CRMEntity.php +++ b/data/CRMEntity.php @@ -206,7 +206,8 @@ class CRMEntity { $upload_file_path = decideFilePath(); // upload the file in server - $upload_status = copy($filetmp_name, $upload_file_path . $current_id . "_" . Vtiger_Util_Helper::getEncryptedFileName($binFile)); + $encryptFileName = Vtiger_Util_Helper::getEncryptedFileName($binFile); + $upload_status = copy($filetmp_name, $upload_file_path . $current_id . "_" . $encryptFileName); // temporary file will be deleted at the end of request if ($save_file == 'true' && $upload_status == 'true') { @@ -231,8 +232,8 @@ class CRMEntity { $params1 = array($current_id, $current_user->id, $ownerid, $module." ".$attachmentType, $this->column_fields['description'], $adb->formatDate($date_var, true), $adb->formatDate($date_var, true)); $adb->pquery($sql1, $params1); //Add entry to attachments - $sql2 = "INSERT INTO vtiger_attachments(attachmentsid, name, description, type, path) values(?, ?, ?, ?, ?)"; - $params2 = array($current_id, $filename, $this->column_fields['description'], $filetype, $upload_file_path); + $sql2 = "INSERT INTO vtiger_attachments(attachmentsid, name, description, type, path, storedname) values(?, ?, ?, ?, ?, ?)"; + $params2 = array($current_id, $filename, $this->column_fields['description'], $filetype, $upload_file_path, $encryptFileName); $adb->pquery($sql2, $params2); //Add relation $sql3 = 'INSERT INTO vtiger_seattachmentsrel VALUES(?,?)'; diff --git a/layouts/v7/modules/Accounts/DetailViewHeaderTitle.tpl b/layouts/v7/modules/Accounts/DetailViewHeaderTitle.tpl index 7b92d5ce0..eedc4447e 100644 --- a/layouts/v7/modules/Accounts/DetailViewHeaderTitle.tpl +++ b/layouts/v7/modules/Accounts/DetailViewHeaderTitle.tpl @@ -15,8 +15,8 @@ <div class="hidden-sm hidden-xs recordImage bgAccounts app-{$SELECTED_MENU_CATEGORY}"> {assign var=IMAGE_DETAILS value=$RECORD->getImageDetails()} {foreach key=ITER item=IMAGE_INFO from=$IMAGE_DETAILS} - {if !empty($IMAGE_INFO.path)} - <img src="{$IMAGE_INFO.path}_{$IMAGE_INFO.orgname}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> + {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.path)} + <img src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> {else} <img src="{vimage_path('summary_organizations.png')}" class="summaryImg"/> {/if} diff --git a/layouts/v7/modules/Invoice/DetailViewHeaderTitle.tpl b/layouts/v7/modules/Invoice/DetailViewHeaderTitle.tpl index 47393217a..00976a9bd 100644 --- a/layouts/v7/modules/Invoice/DetailViewHeaderTitle.tpl +++ b/layouts/v7/modules/Invoice/DetailViewHeaderTitle.tpl @@ -15,8 +15,8 @@ <div class="hidden-sm hidden-xs recordImage bginvoice app-{$SELECTED_MENU_CATEGORY}"> {assign var=IMAGE_DETAILS value=$RECORD->getImageDetails()} {foreach key=ITER item=IMAGE_INFO from=$IMAGE_DETAILS} - {if !empty($IMAGE_INFO.path)} - <img src="{$IMAGE_INFO.path}_{$IMAGE_INFO.orgname}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> + {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.path)} + <img src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> {else} <img src="{vimage_path('summary_organizations.png')}" class="summaryImg"/> {/if} diff --git a/layouts/v7/modules/Leads/DetailViewHeaderTitle.tpl b/layouts/v7/modules/Leads/DetailViewHeaderTitle.tpl index 9339f2059..cf005265f 100644 --- a/layouts/v7/modules/Leads/DetailViewHeaderTitle.tpl +++ b/layouts/v7/modules/Leads/DetailViewHeaderTitle.tpl @@ -15,8 +15,8 @@ <div class="hidden-sm hidden-xs recordImage bgleads app-{$SELECTED_MENU_CATEGORY}"> {assign var=IMAGE_DETAILS value=$RECORD->getImageDetails()} {foreach key=ITER item=IMAGE_INFO from=$IMAGE_DETAILS} - {if !empty($IMAGE_INFO.path)} - <img src="{$IMAGE_INFO.path}_{$IMAGE_INFO.orgname}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100px" align="left"><br> + {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.path)} + <img src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100px" align="left"><br> {else} <img src="{vimage_path('summary_Leads.png')}" class="summaryImg"/> {/if} diff --git a/layouts/v7/modules/PurchaseOrder/DetailViewHeaderTitle.tpl b/layouts/v7/modules/PurchaseOrder/DetailViewHeaderTitle.tpl index a2c8da0ee..88b5d62e0 100644 --- a/layouts/v7/modules/PurchaseOrder/DetailViewHeaderTitle.tpl +++ b/layouts/v7/modules/PurchaseOrder/DetailViewHeaderTitle.tpl @@ -15,8 +15,8 @@ <div class="hidden-sm hidden-xs recordImage bgpurchaseorder app-{$SELECTED_MENU_CATEGORY}"> {assign var=IMAGE_DETAILS value=$RECORD->getImageDetails()} {foreach key=ITER item=IMAGE_INFO from=$IMAGE_DETAILS} - {if !empty($IMAGE_INFO.path)} - <img src="{$IMAGE_INFO.path}_{$IMAGE_INFO.orgname}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> + {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.path)} + <img src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> {else} <img src="{vimage_path('summary_organizations.png')}" class="summaryImg"/> {/if} diff --git a/layouts/v7/modules/Quotes/DetailViewHeaderTitle.tpl b/layouts/v7/modules/Quotes/DetailViewHeaderTitle.tpl index 9fcf58ad1..3ec010d24 100644 --- a/layouts/v7/modules/Quotes/DetailViewHeaderTitle.tpl +++ b/layouts/v7/modules/Quotes/DetailViewHeaderTitle.tpl @@ -15,8 +15,8 @@ <div class="hidden-sm hidden-xs recordImage bgquotes app-{$SELECTED_MENU_CATEGORY}"> {assign var=IMAGE_DETAILS value=$RECORD->getImageDetails()} {foreach key=ITER item=IMAGE_INFO from=$IMAGE_DETAILS} - {if !empty($IMAGE_INFO.path)} - <img src="{$IMAGE_INFO.path}_{$IMAGE_INFO.orgname}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> + {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.path)} + <img src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> {else} <img src="{vimage_path('summary_organizations.png')}" class="summaryImg"/> {/if} diff --git a/layouts/v7/modules/SalesOrder/DetailViewHeaderTitle.tpl b/layouts/v7/modules/SalesOrder/DetailViewHeaderTitle.tpl index 965261dd6..6095458f1 100644 --- a/layouts/v7/modules/SalesOrder/DetailViewHeaderTitle.tpl +++ b/layouts/v7/modules/SalesOrder/DetailViewHeaderTitle.tpl @@ -15,8 +15,8 @@ <div class="hidden-sm hidden-xs recordImage bgsalesorder app-{$SELECTED_MENU_CATEGORY}"> {assign var=IMAGE_DETAILS value=$RECORD->getImageDetails()} {foreach key=ITER item=IMAGE_INFO from=$IMAGE_DETAILS} - {if !empty($IMAGE_INFO.path)} - <img src="{$IMAGE_INFO.path}_{$IMAGE_INFO.orgname}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> + {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.path)} + <img src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" width="100%" height="100%" align="left"><br> {else} <img src="{vimage_path('summary_organizations.png')}" class="summaryImg"/> {/if} diff --git a/layouts/v7/modules/Users/UserViewHeader.tpl b/layouts/v7/modules/Users/UserViewHeader.tpl index 3cef09e74..0e4b62665 100644 --- a/layouts/v7/modules/Users/UserViewHeader.tpl +++ b/layouts/v7/modules/Users/UserViewHeader.tpl @@ -19,7 +19,7 @@ {assign var=NOIMAGE value=0} {foreach key=ITER item=IMAGE_INFO from=$RECORD->getImageDetails()} {if !empty($IMAGE_INFO.url) && !empty($IMAGE_INFO.orgname)} - <img height="100%" width="100%"src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" data-image-id="{$IMAGE_INFO.id}"> + <img height="100%" width="100%" src="{$IMAGE_INFO.url}" alt="{$IMAGE_INFO.orgname}" title="{$IMAGE_INFO.orgname}" data-image-id="{$IMAGE_INFO.id}"> {else} {assign var=NOIMAGE value=1} {/if} diff --git a/layouts/v7/modules/Vtiger/EmailPreview.tpl b/layouts/v7/modules/Vtiger/EmailPreview.tpl index 04c027e9f..db4bce287 100644 --- a/layouts/v7/modules/Vtiger/EmailPreview.tpl +++ b/layouts/v7/modules/Vtiger/EmailPreview.tpl @@ -135,9 +135,9 @@ {foreach item=ATTACHMENT_DETAILS from=$RECORD->getAttachmentDetails()} <i class="fa fa-download"></i> <a {if array_key_exists('docid',$ATTACHMENT_DETAILS)} - href="index.php?module=Documents&action=DownloadFile&record={$ATTACHMENT_DETAILS['docid']}&fileid={$ATTACHMENT_DETAILS['fileid']}" + href="index.php?module=Documents&action=DownloadFile&record={$ATTACHMENT_DETAILS['docid']}&fileid={$ATTACHMENT_DETAILS['fileid']}&name={$ATTACHMENT_DETAILS['attachment']}" {else} - href="index.php?module=Emails&action=DownloadFile&attachment_id={$ATTACHMENT_DETAILS['fileid']}" + href="index.php?module=Emails&action=DownloadFile&attachment_id={$ATTACHMENT_DETAILS['fileid']}&name={$ATTACHMENT_DETAILS['attachment']}" {/if}>{$ATTACHMENT_DETAILS['attachment']}</a> {/foreach} {/if} diff --git a/modules/Documents/models/Record.php b/modules/Documents/models/Record.php index 0fc6b1566..44cd7b29e 100644 --- a/modules/Documents/models/Record.php +++ b/modules/Documents/models/Record.php @@ -21,7 +21,7 @@ class Documents_Record_Model extends Vtiger_Record_Model { function getDownloadFileURL() { if ($this->get('filelocationtype') == 'I') { $fileDetails = $this->getFileDetails(); - return 'index.php?module='. $this->getModuleName() .'&action=DownloadFile&record='. $this->getId() .'&fileid='. $fileDetails['attachmentsid']; + return 'index.php?module='. $this->getModuleName() .'&action=DownloadFile&record='. $this->getId() .'&fileid='. $fileDetails['attachmentsid'].'&name='. $fileDetails['name']; } else { return $this->get('filename'); } @@ -40,8 +40,9 @@ class Documents_Record_Model extends Vtiger_Record_Model { $fileDetails = $this->getFileDetails(); if (!empty ($fileDetails)) { $filePath = $fileDetails['path']; + $storedFileName = $fileDetails['storedname']; - $savedFile = $fileDetails['attachmentsid']."_".decode_html($this->get('filename')); + $savedFile = $fileDetails['attachmentsid']."_".$storedFileName; if(fopen($filePath.$savedFile, "r")) { $returnValue = true; @@ -72,10 +73,11 @@ class Documents_Record_Model extends Vtiger_Record_Model { if (!empty ($fileDetails)) { $filePath = $fileDetails['path']; $fileName = $fileDetails['name']; + $storedFileName = $fileDetails['storedname']; if ($this->get('filelocationtype') == 'I') { $fileName = html_entity_decode($fileName, ENT_QUOTES, vglobal('default_charset')); - $savedFile = $fileDetails['attachmentsid']."_".Vtiger_Util_Helper::getEncryptedFileName($fileName); + $savedFile = $fileDetails['attachmentsid']."_".$storedFileName; while(ob_get_level()) { ob_end_clean(); diff --git a/modules/Emails/actions/DownloadFile.php b/modules/Emails/actions/DownloadFile.php index 9a85bcfca..882a1ddf5 100644 --- a/modules/Emails/actions/DownloadFile.php +++ b/modules/Emails/actions/DownloadFile.php @@ -24,8 +24,9 @@ class Emails_DownloadFile_Action extends Vtiger_Action_Controller { $db = PearDatabase::getInstance(); $attachmentId = $request->get('attachment_id'); - $query = "SELECT * FROM vtiger_attachments WHERE attachmentsid = ?" ; - $result = $db->pquery($query, array($attachmentId)); + $name = $request->get('name'); + $query = "SELECT * FROM vtiger_attachments WHERE attachmentsid = ? AND name = ?" ; + $result = $db->pquery($query, array($attachmentId, $name)); if($db->num_rows($result) == 1) { @@ -34,7 +35,8 @@ class Emails_DownloadFile_Action extends Vtiger_Action_Controller { $name = $row["name"]; $filepath = $row["path"]; $name = decode_html($name); - $saved_filename = $attachmentId."_". Vtiger_Util_Helper::getEncryptedFileName($name); + $storedFileName = $row['storedname']; + $saved_filename = $attachmentId."_". $storedFileName; $disk_file_size = filesize($filepath.$saved_filename); $filesize = $disk_file_size + ($disk_file_size % 1024); $fileContent = fread(fopen($filepath.$saved_filename, "r"), $filesize); diff --git a/modules/Migration/schema/711_to_720.php b/modules/Migration/schema/711_to_720.php new file mode 100644 index 000000000..fa7be8943 --- /dev/null +++ b/modules/Migration/schema/711_to_720.php @@ -0,0 +1,17 @@ +<?php +/*+******************************************************************************** + * The contents of this file are subject to the vtiger CRM Public License Version 1.0 + * ("License"); You may not use this file except in compliance with the License + * The Original Code is: vtiger CRM Open Source + * The Initial Developer of the Original Code is vtiger. + * Portions created by vtiger are Copyright (C) vtiger. + * All Rights Reserved. + *********************************************************************************/ + +if (defined('VTIGER_UPGRADE')) { + global $current_user, $adb; + $db = PearDatabase::getInstance(); + + // Added column storedname for vtiger_attachments to support reverse mapping. + $db->pquery('ALTER TABLE vtiger_attachments ADD COLUMN storedname varchar(255) NOT NULL AFTER path', array()); +} diff --git a/modules/Users/Users.php b/modules/Users/Users.php index bbd76dad3..b8c183aa4 100755 --- a/modules/Users/Users.php +++ b/modules/Users/Users.php @@ -1058,7 +1058,8 @@ class Users extends CRMEntity { //get the file path inwhich folder we want to upload the file $upload_file_path = decideFilePath(); //upload the file in server - $upload_status = move_uploaded_file($filetmp_name,$upload_file_path.$current_id."_".Vtiger_Util_Helper::getEncryptedFileName($binFile)); + $encryptFileName = Vtiger_Util_Helper::getEncryptedFileName($binFile); + $upload_status = move_uploaded_file($filetmp_name,$upload_file_path.$current_id."_".$encryptFileName); if($save_file == 'true') { @@ -1066,8 +1067,8 @@ class Users extends CRMEntity { $params1 = array($current_id, $current_user->id, $ownerid, $module." Image", $this->column_fields['description'], $this->db->formatString("vtiger_crmentity","createdtime",$date_var), $this->db->formatDate($date_var, true)); $this->db->pquery($sql1, $params1); - $sql2="insert into vtiger_attachments(attachmentsid, name, description, type, path) values(?,?,?,?,?)"; - $params2 = array($current_id, $filename, $this->column_fields['description'], $filetype, $upload_file_path); + $sql2="insert into vtiger_attachments(attachmentsid, name, description, type, path, storedname) values(?,?,?,?,?,?)"; + $params2 = array($current_id, $filename, $this->column_fields['description'], $filetype, $upload_file_path, $encryptFileName); $result=$this->db->pquery($sql2, $params2); if($id != '') { diff --git a/modules/Vtiger/helpers/ShowFile.php b/modules/Vtiger/helpers/ShowFile.php index 856724397..8baa67c7a 100644 --- a/modules/Vtiger/helpers/ShowFile.php +++ b/modules/Vtiger/helpers/ShowFile.php @@ -21,13 +21,14 @@ class Vtiger_ShowFile_Helper { $query = "SELECT vtiger_attachments.* FROM vtiger_attachments INNER JOIN vtiger_crmentity ON vtiger_crmentity.crmid = vtiger_attachments.attachmentsid - WHERE vtiger_attachments.attachmentsid=? LIMIT 1"; - $result = $db->pquery($query, array($fid)); + WHERE vtiger_attachments.attachmentsid=? AND vtiger_attachments.name=? LIMIT 1"; + $result = $db->pquery($query, array($fid, $encFileName)); if ($result && $db->num_rows($result)) { $resultData = $db->fetch_array($result); $fileId = $resultData['attachmentsid']; $filePath = $resultData['path']; $fileName = $resultData['name']; + $storedFileName = $resultData['storedname']; $fileType = $resultData['type']; $sanitizedFileName = sanitizeUploadFileName($fileName, $upload_badext); @@ -35,8 +36,8 @@ class Vtiger_ShowFile_Helper { * While saving the document applying decode_html to save in DB, but this is not happening for the images * This save happens from mailroom, inbox, record save, document save etc.. */ - if (md5($fileName) == $encFileName || md5($sanitizedFileName) == $encFileName) { - $finalFilePath = $filePath.$fileId.'_'.Vtiger_Util_Helper::getEncryptedFileName($sanitizedFileName); + if (!empty($encFileName) && !empty($storedFileName)) { + $finalFilePath = $filePath.$fileId.'_'.$storedFileName; $isFileExist = false; if (file_exists($finalFilePath)) { $isFileExist = true; diff --git a/modules/Vtiger/helpers/Util.php b/modules/Vtiger/helpers/Util.php index 4d793d451..3d330a579 100644 --- a/modules/Vtiger/helpers/Util.php +++ b/modules/Vtiger/helpers/Util.php @@ -1244,7 +1244,7 @@ class Vtiger_Util_Helper { if ($sanitizedFileName) { $fileNameParts = explode('.', decode_html($sanitizedFileName)); $fileType = array_pop($fileNameParts); - $encryptedFileName = md5(implode('.', $fileNameParts)).'.'.$fileType; + $encryptedFileName = md5(md5(microtime(true)).implode('.', $fileNameParts)).'.'.$fileType; } return $encryptedFileName; } diff --git a/modules/Vtiger/models/Record.php b/modules/Vtiger/models/Record.php index b091aceef..0a5662602 100644 --- a/modules/Vtiger/models/Record.php +++ b/modules/Vtiger/models/Record.php @@ -591,8 +591,9 @@ class Vtiger_Record_Model extends Vtiger_Base_Model { if (!empty ($fileDetails)) { $filePath = $fileDetails['path']; $fileName = $fileDetails['name']; + $storedFileName = $fileDetails['storedname']; $fileName = html_entity_decode($fileName, ENT_QUOTES, vglobal('default_charset')); - $savedFile = $fileDetails['attachmentsid']."_".Vtiger_Util_Helper::getEncryptedFileName($fileName); + $savedFile = $fileDetails['attachmentsid']."_".$storedFileName; $fileSize = filesize($filePath.$savedFile); $fileSize = $fileSize + ($fileSize % 1024); if (fopen($filePath.$savedFile, "r")) { @@ -600,7 +601,7 @@ class Vtiger_Record_Model extends Vtiger_Base_Model { header("Content-type: ".$fileDetails['type']); header("Pragma: public"); header("Cache-Control: private"); - header("Content-Disposition: attachment; filename=\"$fileName\""); + header("Content-Disposition: attachment; filename=\"$savedFile\""); header("Content-Description: PHP Generated Data"); header("Content-Encoding: none"); } diff --git a/vtlib/Vtiger/Functions.php b/vtlib/Vtiger/Functions.php index 38bb8079b..840421bb8 100644 --- a/vtlib/Vtiger/Functions.php +++ b/vtlib/Vtiger/Functions.php @@ -1519,7 +1519,7 @@ class Vtiger_Functions { $fileId = $imageId; $fileName = $imageName; if ($fileId) { - $publicUrl = "public.php?fid=$fileId&key=".md5($fileName); + $publicUrl = "public.php?fid=$fileId&key=".$fileName; } return $publicUrl; } -- GitLab