From 8538fffd6679c800cf473ddd3ce35a8d41d6681e Mon Sep 17 00:00:00 2001 From: dante Date: Mon, 7 Aug 2017 13:11:45 -0400 Subject: [PATCH 1/3] HOR-3434 --- workflow/engine/controllers/pmTables.php | 35 ++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/workflow/engine/controllers/pmTables.php b/workflow/engine/controllers/pmTables.php index 4d1099d80..0d0d40d46 100644 --- a/workflow/engine/controllers/pmTables.php +++ b/workflow/engine/controllers/pmTables.php @@ -152,6 +152,11 @@ class pmTables extends Controller $sFileName = $httpData->f; $realPath = $PUBLIC_ROOT_PATH . $sFileName; + + if ($this->isValidFileToBeStreamed($realPath, $PUBLIC_ROOT_PATH) === false) { + throw new Exception("You are trying to access an unauthorized resource."); + } + G::streamFile( $realPath, true ); unlink( $realPath ); } @@ -206,5 +211,35 @@ class pmTables extends Controller $tableSize = $tableSize - 8; // Prefix PMT_ return $tableSize; } + + /** + * Validates if the file with the path $filePath is a valid one, + * that is, it must be a file within the temporal directory where the + * exported pmt files are created and must have one of the valid file + * extensions. + * + * @param $filePath, full path to the temporal file that will be streamed + * @param $tempDir, directory's path where the temporal files are created. + * @return bool + */ + private function isValidFileToBeStreamed($filePath, $tempDir) + { + $result = true; + $validExtensionsForExporting = ['csv', 'pmt']; + $fileRealPath = realpath($filePath); + $tempDirRealPath = realpath($tempDir); + + $pathInfo = pathinfo($fileRealPath); + + if ($pathInfo ['dirname'] !== $tempDirRealPath) { + $result = false; + } + + if (!in_array($pathInfo['extension'], $validExtensionsForExporting)) { + $result = false; + } + + return $result; + } } From e41d6d460f3c7a9912092ea196eb2a126d53bdf6 Mon Sep 17 00:00:00 2001 From: dante Date: Tue, 8 Aug 2017 08:35:40 -0400 Subject: [PATCH 2/3] Using just pathinfo to validate that a file name and not a relative path \was sent --- workflow/engine/controllers/pmTables.php | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/workflow/engine/controllers/pmTables.php b/workflow/engine/controllers/pmTables.php index 0d0d40d46..0bd128295 100644 --- a/workflow/engine/controllers/pmTables.php +++ b/workflow/engine/controllers/pmTables.php @@ -153,7 +153,7 @@ class pmTables extends Controller $realPath = $PUBLIC_ROOT_PATH . $sFileName; - if ($this->isValidFileToBeStreamed($realPath, $PUBLIC_ROOT_PATH) === false) { + if ($this->isValidFileToBeStreamed($sFileName) === false) { throw new Exception("You are trying to access an unauthorized resource."); } @@ -213,25 +213,22 @@ class pmTables extends Controller } /** - * Validates if the file with the path $filePath is a valid one, - * that is, it must be a file within the temporal directory where the - * exported pmt files are created and must have one of the valid file - * extensions. + * Validates if the file with the $fileName is a valid one, + * that is, it must be a file without relative references that + * can open a door to get some unauthorized system file and + * must have one of the valid file extensions. * - * @param $filePath, full path to the temporal file that will be streamed - * @param $tempDir, directory's path where the temporal files are created. + * @param $fileName, emporal file name that will be streamed * @return bool */ - private function isValidFileToBeStreamed($filePath, $tempDir) + private function isValidFileToBeStreamed($fileName) { $result = true; $validExtensionsForExporting = ['csv', 'pmt']; - $fileRealPath = realpath($filePath); - $tempDirRealPath = realpath($tempDir); - $pathInfo = pathinfo($fileRealPath); + $pathInfo = pathinfo($fileName); - if ($pathInfo ['dirname'] !== $tempDirRealPath) { + if ($pathInfo ['dirname'] !== '.') { $result = false; } From c7e08398f91cd4c0e44ec2a5b27ecc652206906b Mon Sep 17 00:00:00 2001 From: dante Date: Tue, 8 Aug 2017 09:57:22 -0400 Subject: [PATCH 3/3] delete of extra space beteween variable and index bracket --- workflow/engine/controllers/pmTables.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/engine/controllers/pmTables.php b/workflow/engine/controllers/pmTables.php index 0bd128295..5ed01b43d 100644 --- a/workflow/engine/controllers/pmTables.php +++ b/workflow/engine/controllers/pmTables.php @@ -228,7 +228,7 @@ class pmTables extends Controller $pathInfo = pathinfo($fileName); - if ($pathInfo ['dirname'] !== '.') { + if ($pathInfo['dirname'] !== '.') { $result = false; }