From 053d8e754e9335945c5a1b7e3ba53930ba54660f Mon Sep 17 00:00:00 2001 From: cfreal Date: Mon, 16 Nov 2020 11:23:24 +0100 Subject: [PATCH 1/4] Disallow paths with a scheme in MakeViewVariableOptionalSolution --- src/Solutions/MakeViewVariableOptionalSolution.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Solutions/MakeViewVariableOptionalSolution.php b/src/Solutions/MakeViewVariableOptionalSolution.php index 5e4164de..295a6487 100644 --- a/src/Solutions/MakeViewVariableOptionalSolution.php +++ b/src/Solutions/MakeViewVariableOptionalSolution.php @@ -4,6 +4,7 @@ use Facade\IgnitionContracts\RunnableSolution; use Illuminate\Support\Facades\Blade; +use Illuminate\Support\Str; class MakeViewVariableOptionalSolution implements RunnableSolution { @@ -72,6 +73,10 @@ public function run(array $parameters = []) public function makeOptional(array $parameters = []) { + # Disregard schemed paths such as ftp://, phar://, etc. + if (!Str::startsWith($parameters['viewFile'], '/') || !Str::endsWith($parameters['viewFile'], '.blade.php')) { + return false; + } $originalContents = file_get_contents($parameters['viewFile']); $newContents = str_replace('$'.$parameters['variableName'], '$'.$parameters['variableName']." ?? ''", $originalContents); From 8aee24df98acbe917ae5983f8631440bd0ec8c8f Mon Sep 17 00:00:00 2001 From: cfreal Date: Mon, 16 Nov 2020 12:10:13 +0100 Subject: [PATCH 2/4] Added a test unit for the solution --- .../MakeViewVariableOptionalSolution.php | 6 +++ .../MakeViewVariableOptionalSolutionTest.php | 53 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 tests/Solutions/MakeViewVariableOptionalSolutionTest.php diff --git a/src/Solutions/MakeViewVariableOptionalSolution.php b/src/Solutions/MakeViewVariableOptionalSolution.php index 5e4164de..2ac96bca 100644 --- a/src/Solutions/MakeViewVariableOptionalSolution.php +++ b/src/Solutions/MakeViewVariableOptionalSolution.php @@ -4,6 +4,7 @@ use Facade\IgnitionContracts\RunnableSolution; use Illuminate\Support\Facades\Blade; +use Illuminate\Support\Str; class MakeViewVariableOptionalSolution implements RunnableSolution { @@ -72,6 +73,11 @@ public function run(array $parameters = []) public function makeOptional(array $parameters = []) { + # Disregard schemed paths such as ftp://, phar://, etc. + if (!Str::startsWith($parameters['viewFile'], ['/', './']) || !Str::endsWith($parameters['viewFile'], '.blade.php')) { + return false; + } + $originalContents = file_get_contents($parameters['viewFile']); $newContents = str_replace('$'.$parameters['variableName'], '$'.$parameters['variableName']." ?? ''", $originalContents); diff --git a/tests/Solutions/MakeViewVariableOptionalSolutionTest.php b/tests/Solutions/MakeViewVariableOptionalSolutionTest.php new file mode 100644 index 00000000..9c28023e --- /dev/null +++ b/tests/Solutions/MakeViewVariableOptionalSolutionTest.php @@ -0,0 +1,53 @@ +app->bind( + ComposerClassMap::class, + function () { + return new ComposerClassMap(__DIR__.'/../../vendor/autoload.php'); + } + ); + } + + /** @test */ + public function it_does_not_open_scheme_paths() + { + $solution = $this->getSolutionForPath('php://filter/resource=./tests/stubs/views/blade-exception.blade.php'); + $this->assertFalse($solution->isRunnable()); + } + + /** @test */ + public function it_does_open_relative_paths() + { + $solution = $this->getSolutionForPath('./tests/stubs/views/blade-exception.blade.php'); + $this->assertTrue($solution->isRunnable()); + } + + /** @test */ + public function it_does_not_open_other_extentions() + { + $solution = $this->getSolutionForPath('./tests/stubs/views/php-exception.php'); + $this->assertFalse($solution->isRunnable()); + } + + protected function getSolutionForPath($path): MakeViewVariableOptionalSolution + { + return new MakeViewVariableOptionalSolution('notSet', $path); + } +} From a92f9b1a282824d15ed37b381a3502d326e1465b Mon Sep 17 00:00:00 2001 From: cfreal Date: Tue, 17 Nov 2020 09:57:16 +0100 Subject: [PATCH 3/4] Refactored code into a isSafePath() method --- .../MakeViewVariableOptionalSolution.php | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Solutions/MakeViewVariableOptionalSolution.php b/src/Solutions/MakeViewVariableOptionalSolution.php index 72324afe..bbb9f651 100644 --- a/src/Solutions/MakeViewVariableOptionalSolution.php +++ b/src/Solutions/MakeViewVariableOptionalSolution.php @@ -71,10 +71,26 @@ public function run(array $parameters = []) } } + /** + * Verifies that $path is either an absolute or a relative path, and ends with .blade.php + * @param string $path + * @return bool + */ + protected function isSafePath(string $path) + { + if (!Str::startsWith($path, ['/', './'])) { + return false; + } + if (!Str::endsWith($path, '.blade.php')) { + return false; + } + + return true; + } + public function makeOptional(array $parameters = []) { - # Only allow full or relative paths, and paths that end in .blade.php - if (!Str::startsWith($parameters['viewFile'], ['/', './']) || !Str::endsWith($parameters['viewFile'], '.blade.php')) { + if (!$this->isSafePath($parameters['viewFile'])) { return false; } From 98bc52835db2aa804d5891cce71ef3cf71b4257e Mon Sep 17 00:00:00 2001 From: Freek Van der Herten Date: Tue, 17 Nov 2020 10:09:30 +0100 Subject: [PATCH 4/4] Update MakeViewVariableOptionalSolution.php --- src/Solutions/MakeViewVariableOptionalSolution.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Solutions/MakeViewVariableOptionalSolution.php b/src/Solutions/MakeViewVariableOptionalSolution.php index bbb9f651..3a685d0b 100644 --- a/src/Solutions/MakeViewVariableOptionalSolution.php +++ b/src/Solutions/MakeViewVariableOptionalSolution.php @@ -71,12 +71,7 @@ public function run(array $parameters = []) } } - /** - * Verifies that $path is either an absolute or a relative path, and ends with .blade.php - * @param string $path - * @return bool - */ - protected function isSafePath(string $path) + protected function isSafePath(string $path): bool { if (!Str::startsWith($path, ['/', './'])) { return false;