From 65d060f5271ab4db41c851f73b81e65a27356d4e Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 12 Feb 2019 15:58:57 -0800 Subject: [PATCH] generateFileHash() - If we can't generate a secure, then don't generate any token Falling back to a constant negates any security benefit of using a hash. IMHO, the edge-case where `CIVICRM_SITE_KEY` is missing should be obscure/rare and signifies broader problems for the deployment. It needs to be corrected. If you're worried that having an error-symptom here is too obscure, then let's add a more prominent error-message via `CRM_Utils_Check`. NOTE: There is one pre-existing case in core where (in absence of a key) it procedes with a constant in lieu of a `CIVICRM_SITE_KEY` . Specifically, `CRM_Core_Error::generateLogFileHash()`. That is not a good example to follow because it is qualitiatively different: * In `generateLogFileHash`(), `CIVICRM_SITE_KEY` functions as one of multiple redundant security mechanisms -- e.g. even if `CIVICRM_SITE_KEY` is missing, the log file remains hard-to-access because (1) the DSN is part of the hash and (2) the httpd protects `ConfigAndLog`. (Contrast: The file-hash-code is not *redundant* in the same way.) * In the context of logging, raising any error (even if it's real error condition) can provoke a weird loop (because then that error needs to be logged). The log needs to avoid such loops. (Contrast: `generateFileHash()` is part of the normal post-boot application logic, so it's free to register errors normally.) --- CRM/Core/BAO/File.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CRM/Core/BAO/File.php b/CRM/Core/BAO/File.php index 4468dd8c0f..30e26b9a29 100644 --- a/CRM/Core/BAO/File.php +++ b/CRM/Core/BAO/File.php @@ -775,7 +775,10 @@ AND CEF.entity_id = %2"; */ public static function generateFileHash($eid = NULL, $fid = NULL, $genTs = NULL, $life = NULL) { // Use multiple (but stable) inputs for hash information. - $siteKey = defined('CIVICRM_SITE_KEY') ? CIVICRM_SITE_KEY : 'NO_SITE_KEY'; + $siteKey = CRM_Utils_Constant::value('CIVICRM_SITE_KEY'); + if (!$siteKey) { + throw new \CRM_Core_Exception("Cannot generate file access token. Please set CIVICRM_SITE_KEY."); + } if (!$genTs) { $genTs = time(); -- 2.25.1