From efddd94fdbfa4f63e1d9211faecbc750f78b447d Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 13 Feb 2019 12:50:02 -0800
Subject: [PATCH] Fix multiple issues with file URLs. Use clearer variables and
 docblocks to reduce confusion.

---
 CRM/Core/BAO/CustomField.php |  2 +-
 CRM/Core/BAO/CustomGroup.php |  2 +-
 CRM/Core/BAO/File.php        | 10 +++++-----
 CRM/Core/Page/File.php       |  8 ++++----
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php
index 5f2c432773..0fb865d4d8 100644
--- a/CRM/Core/BAO/CustomField.php
+++ b/CRM/Core/BAO/CustomField.php
@@ -1495,7 +1495,7 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField {
             'file_id'
           );
           list($path) = CRM_Core_BAO_File::path($fileID, $entityId);
-          $fileHash = CRM_Core_BAO_File::generateFileHash($eid, $fileID);
+          $fileHash = CRM_Core_BAO_File::generateFileHash($entityId, $fileID);
           $url = CRM_Utils_System::url('civicrm/file',
             "reset=1&id=$fileID&eid=$contactID&fcs=$fileHash",
             $absolute, NULL, TRUE, TRUE
diff --git a/CRM/Core/BAO/CustomGroup.php b/CRM/Core/BAO/CustomGroup.php
index c10e09218c..df5d09adaf 100644
--- a/CRM/Core/BAO/CustomGroup.php
+++ b/CRM/Core/BAO/CustomGroup.php
@@ -874,8 +874,8 @@ ORDER BY civicrm_custom_group.weight,
         $fileDAO->id = $dao->$fieldName;
 
         if ($fileDAO->find(TRUE)) {
-          $fileHash = CRM_Core_BAO_File::generateFileHash($dao->$entityIDName, $fileDAO->id);
           $entityIDName = "{$table}_entity_id";
+          $fileHash = CRM_Core_BAO_File::generateFileHash($dao->$entityIDName, $fileDAO->id);
           $customValue['id'] = $dao->$idName;
           $customValue['data'] = $fileDAO->uri;
           $customValue['fid'] = $fileDAO->id;
diff --git a/CRM/Core/BAO/File.php b/CRM/Core/BAO/File.php
index a7fcc5fbd9..e33984412f 100644
--- a/CRM/Core/BAO/File.php
+++ b/CRM/Core/BAO/File.php
@@ -769,11 +769,11 @@ AND       CEF.entity_id    = %2";
 
   /**
    * Generates a MD5 Hash to be appended to file URLS to be checked when trying to download the file.
-   * @param int $eid entity id the file is attached to
-   * @param int $fid file ID
+   * @param int $entityId entity id the file is attached to
+   * @param int $fileId file ID
    * @return string
    */
-  public static function generateFileHash($eid = NULL, $fid = NULL, $genTs = NULL, $life = NULL) {
+  public static function generateFileHash($entityId = NULL, $fileId = NULL, $genTs = NULL, $life = NULL) {
     // Use multiple (but stable) inputs for hash information.
     $siteKey = CRM_Utils_Constant::value('CIVICRM_SITE_KEY');
     if (!$siteKey) {
@@ -785,11 +785,11 @@ AND       CEF.entity_id    = %2";
     }
     if (!$life) {
       $days = Civi::settings()->get('checksum_timeout');
-      $live = 24 * $days;
+      $life = 24 * $days;
     }
     // Trim 8 chars off the string, make it slightly easier to find
     // but reveals less information from the hash.
-    $cs = hash_hmac('sha256', "{$fid}_{$life}", $siteKey);
+    $cs = hash_hmac('sha256', "entity={$entityId}&file={$fileId}&life={$life}", $siteKey);
     return "{$cs}_{$genTs}_{$life}";
   }
 
diff --git a/CRM/Core/Page/File.php b/CRM/Core/Page/File.php
index dec65cc44c..87a2e4433e 100644
--- a/CRM/Core/Page/File.php
+++ b/CRM/Core/Page/File.php
@@ -42,11 +42,11 @@ class CRM_Core_Page_File extends CRM_Core_Page {
     $download = CRM_Utils_Request::retrieve('download', 'Integer', $this, FALSE, 1);
     $disposition = $download == 0 ? 'inline' : 'download';
 
-    $eid = CRM_Utils_Request::retrieve('eid', 'Positive', $this, TRUE);
-    $fid = CRM_Utils_Request::retrieve('fid', 'Positive', $this, FALSE);
-    $id = CRM_Utils_Request::retrieve('id', 'Positive', $this, TRUE);
+    $eid = CRM_Utils_Request::retrieve('eid', 'Positive', $this, TRUE); // Entity ID (e.g. Contact ID)
+    $fid = CRM_Utils_Request::retrieve('fid', 'Positive', $this, FALSE); // Field ID
+    $id = CRM_Utils_Request::retrieve('id', 'Positive', $this, TRUE); // File ID
     $hash = CRM_Utils_Request::retrieve('fcs', 'Alphanumeric', $this);
-    if (!CRM_Core_BAO_File::validateFileHash($hash, $eid, $fid)) {
+    if (!CRM_Core_BAO_File::validateFileHash($hash, $eid, $id)) {
       CRM_Core_Error::statusBounce('URL for file is not valid');
     }
 
-- 
2.25.1