Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix MakeViewVariableOptionalSolution to disallow stream wrappers and files that do not end in .blade.php #334

Merged

Conversation

cfreal
Copy link
Contributor

@cfreal cfreal commented Nov 16, 2020

Hello, as discussed by email, this fixes a serious vulnerability.
Hopefully my code is OK-ish.

@freekmurze freekmurze merged commit 03a8aa1 into facade:master Nov 17, 2020
@freekmurze
Copy link
Collaborator

Thanks!

@cfreal cfreal deleted the fix-MakeViewVariableOptionalSolution-rce branch January 11, 2021 09:51
@abergmann
Copy link

CVE-2021-3129 was assigned to this PR.

@jplana-ubc
Copy link

Can we get this patch committed to the 1.16.x branch?

@jplana-ubc
Copy link

jplana-ubc commented Jan 28, 2021

I would love to create a PR myself with the changes for the 1.16.4 release, but unfortunately everything lives in the same branch (both 2.x and 1.x versions of this package).

It would be fantastic if there was a way to push these changes to a 1.16.5 tag, so all the folks still using Laravel 6 and facade/ignition 1.x could benefit of this change.

diff --git a/src/Solutions/MakeViewVariableOptionalSolution.php b/src/Solutions/MakeViewVariableOptionalSolution.php
index 2903414..7e53117 100644
--- a/src/Solutions/MakeViewVariableOptionalSolution.php
+++ b/src/Solutions/MakeViewVariableOptionalSolution.php
@@ -4,6 +4,7 @@ namespace Facade\Ignition\Solutions;

 use Facade\IgnitionContracts\RunnableSolution;
 use Illuminate\Support\Facades\Blade;
+use Illuminate\Support\Str;

 class MakeViewVariableOptionalSolution implements RunnableSolution
 {
@@ -71,8 +72,24 @@ class MakeViewVariableOptionalSolution implements RunnableSolution
         }
     }

+    protected function isSafePath(string $path): bool
+    {
+        if (!Str::startsWith($path, ['/', './'])) {
+            return false;
+        }
+        if (!Str::endsWith($path, '.blade.php')) {
+            return false;
+        }
+
+        return true;
+    }
+
     public function makeOptional(array $parameters = [])
     {
+        if (!$this->isSafePath($parameters['viewFile'])) {
+            return false;
+        }
+
         $originalContents = file_get_contents($parameters['viewFile']);
         $newContents = str_replace('$'.$parameters['variableName'], '$'.$parameters['variableName']." ?? ''", $originalContents);

anasmirza534 added a commit to anasmirza534/ignition that referenced this pull request Feb 18, 2021
…files that do not end in .blade.php

This is already fixed in 2.5.2, See facade#334

I could not update to 2.5.2 due to some dependent package required php 7.3, currently clients site is running in php 7.2

On branch 2.4.1-branch
Changes to be committed:
	modified:   src/Solutions/MakeViewVariableOptionalSolution.php
particleflux added a commit to particleflux/security-advisories that referenced this pull request Mar 26, 2021
The vulnerability in `facade/ignition` is falsely reported for version
2.4.2.

It has been fixed in facade/ignition#334 for
2.5.x, and in facade/ignition#356 for 2.4.x.
particleflux added a commit to particleflux/security-advisories that referenced this pull request Mar 26, 2021
The vulnerability in `facade/ignition` is falsely reported for version
2.4.2.

It has been fixed in facade/ignition#334 for
2.5.x, and in facade/ignition#356 for 2.4.x.

Fixes FriendsOfPHP#543
lisadeloach63 pushed a commit to lisadeloach63/laravel-ignition-error-page that referenced this pull request Sep 1, 2022
…files that do not end in .blade.php

This is already fixed in 2.5.2, See facade/ignition#334

I could not update to 2.5.2 due to some dependent package required php 7.3, currently clients site is running in php 7.2

On branch 2.4.1-branch
Changes to be committed:
	modified:   src/Solutions/MakeViewVariableOptionalSolution.php
sadafrangian3 added a commit to sadafrangian3/laravel-ignition that referenced this pull request Apr 6, 2023
…files that do not end in .blade.php

This is already fixed in 2.5.2, See facade/ignition#334

I could not update to 2.5.2 due to some dependent package required php 7.3, currently clients site is running in php 7.2

On branch 2.4.1-branch
Changes to be committed:
	modified:   src/Solutions/MakeViewVariableOptionalSolution.php
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants