From a7762e79566d58c3bc965cc629fd97a361441003 Mon Sep 17 00:00:00 2001
From: Seamus Lee <seamuslee001@gmail.com>
Date: Sat, 23 Feb 2019 08:00:43 +1100
Subject: [PATCH] Extract checking of filename into own function and add tests

---
 CRM/Core/Page/File.php               |  2 +-
 CRM/Utils/File.php                   | 19 +++++++++++++++++++
 tests/phpunit/CRM/Utils/FileTest.php | 21 +++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/CRM/Core/Page/File.php b/CRM/Core/Page/File.php
index 04c5a28d74..e3b12d2a92 100644
--- a/CRM/Core/Page/File.php
+++ b/CRM/Core/Page/File.php
@@ -59,7 +59,7 @@ class CRM_Core_Page_File extends CRM_Core_Page {
       list($path, $mimeType) = CRM_Core_BAO_File::path($fileId, $entityId);
     }
     else {
-      if ($fileName !== basename($fileName)) {
+      if (!CRM_Utils_File::isValidFileName($fileName)) {
         throw new CRM_Core_Exception("Malformed filename");
       }
       $mimeType = '';
diff --git a/CRM/Utils/File.php b/CRM/Utils/File.php
index 34eba794ab..d5d322f8c8 100644
--- a/CRM/Utils/File.php
+++ b/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;
+  }
+
 }
diff --git a/tests/phpunit/CRM/Utils/FileTest.php b/tests/phpunit/CRM/Utils/FileTest.php
index 5e1659d93b..8a6a4eb674 100644
--- a/tests/phpunit/CRM/Utils/FileTest.php
+++ b/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));
+  }
+
 }
-- 
2.25.1