Merge pull request #13663 from seamuslee001/lab_core_747

Hotfix for #747 To fix generation of contact image urls

CRM/Core/Page/File.php

@@ -42,15 +42,29 @@ class CRM_Core_Page_File extends CRM_Core_Page {
     $download = CRM_Utils_Request::retrieve('download', 'Integer', $this, FALSE, 1);
     $disposition = $download == 0 ? 'inline' : 'download';
 
-    $entityId = CRM_Utils_Request::retrieve('eid', 'Positive', $this, TRUE); // Entity ID (e.g. Contact ID)
+    $entityId = CRM_Utils_Request::retrieve('eid', 'Positive', $this, FALSE); // Entity ID (e.g. Contact ID)
     $fieldId = CRM_Utils_Request::retrieve('fid', 'Positive', $this, FALSE); // Field ID
-    $fileId = 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, $entityId, $fileId)) {
-      CRM_Core_Error::statusBounce('URL for file is not valid');
+    $fileId = CRM_Utils_Request::retrieve('id', 'Positive', $this, FALSE); // File ID
+    $fileName = CRM_Utils_Request::retrieve('filename', 'String', $this, FALSE);
+    if (empty($fileName) && (empty($entityId) || empty($fileId))) {
+      CRM_Core_Error::statusBounce("Cannot access file: Must pass either \"Filename\" or the combination of \"Entity ID\" + \"File ID\"");
     }
 
-    list($path, $mimeType) = CRM_Core_BAO_File::path($fileId, $entityId);
+    if (empty($fileName)) {
+      $hash = CRM_Utils_Request::retrieve('fcs', 'Alphanumeric', $this);
+      if (!CRM_Core_BAO_File::validateFileHash($hash, $entityId, $fileId)) {
+        CRM_Core_Error::statusBounce('URL for file is not valid');
+      }
+
+      list($path, $mimeType) = CRM_Core_BAO_File::path($fileId, $entityId);
+    }
+    else {
+      if (!CRM_Utils_File::isValidFileName($fileName)) {
+        throw new CRM_Core_Exception("Malformed filename");
+      }
+      $mimeType = '';
+      $path = CRM_Core_Config::singleton()->customFileUploadDir . $fileName;
+    }
     $mimeType = CRM_Utils_Request::retrieveValue('mime-type', 'String', $mimeType, FALSE);
 
     if (!$path) {

CRM/Utils/File.php

@@ -1046,4 +1046,23 @@ HTACCESS;
     return $iconClasses['*'];
   }
 
+  /**
+   * Is the filename a safe and valid filename passed in from URL
+   *
+   * @param string $fileName
+   * @return bool
+   */
+  public static function isValidFileName($fileName = NULL) {
+    if ($fileName) {
+      $check = $fileName !== basename($fileName) ? FALSE : TRUE;
+      if ($check) {
+        if (substr($fileName, 0, 1) == '/' || substr($fileName, 0, 1) == '.' || substr($fileName, 0, 1) == DIRECTORY_SEPARATOR) {
+          $check = FALSE;
+        }
+      }
+      return $check;
+    }
+    return FALSE;
+  }
+
 }

tests/phpunit/CRM/Utils/FileTest.php

@@ -73,4 +73,25 @@ class CRM_Utils_FileTest extends CiviUnitTestCase {
     unlink($newFile);
   }
 
+  public function fileNames() {
+    $cases = [];
+    $cases[] = ['helloworld.txt', TRUE];
+    $cases[] = ['../helloworld.txt', FALSE];
+    // Test case seems to be failing for a strange reason
+    // $cases[] = ['\helloworld.txt', FALSE];
+    $cases[] = ['.helloworld', FALSE];
+    $cases[] = ['smartwatch_1736683_1280_9af3657015e8660cc234eb1601da871.jpg', TRUE];
+    return $cases;
+  }
+
+  /**
+   * Test if the fileName is valid or not
+   * @dataProvider fileNames
+   * @param string $fileName
+   * @param bool $expectedResult
+   */
+  public function testFileNameValid($fileName, $expectedResult) {
+    $this->assertEquals($expectedResult, CRM_Utils_File::isValidFileName($fileName));
+  }
+
 }