Skip to content
This repository has been archived by the owner on Dec 9, 2019. It is now read-only.

Add a check for phar in stream wrappers. #24

Conversation

jbarton123
Copy link
Contributor

@jbarton123 jbarton123 commented Mar 28, 2019

This resolves an issue whereby the phpstan phar would try to be loaded even if
phar has not been registered as a stream wrapper in PHP.

This is prevalant in Magento >=2.2.8 as there now exists a line which unregisters 'phar' from stream wrappers in the bootstrap.php file.

https://magento.com/security/patches/magento-2.3.1-2.2.8-and-2.1.17-security-update (PRODSECBUG-2261: Arbitrary code execution due to unsafe deserialization of a PHP archive)

If PhpStan is loaded via the composer autoloader it causes magento commands to fail with this error -

PHP Warning:  require_once(): Unable to find the wrapper "phar" - did you forget to enable it when you configured PHP? in /app/vendor/phpstan/phpstan-shim/bootstrap.php on line 4

This resolves an issue whereby the phpstan phar would try to be loaded even if
phar has not been registered as a stream wrapper in PHP.
@jbarton123 jbarton123 changed the title Add a check for phar in the stream wrappers. Add a check for phar in stream wrappers. Mar 28, 2019
@ondrejmirtes
Copy link
Member

Hi, is this related to magento/magento2#21973? I think that this isn't right, because it will cause other things to silently fail. This bootstrap is here because if you install phpstan-shim and some other thing that needs nikic/php-parser, the PHP-Parser is loaded from PHPStan's PHAR thanks to this line. So if you're in this situation with unregistered stream wrapper, you will break something else and won't even know why.

I'd rather have some exception thrown from the bootstrap if the phar:// wrapper isn't registered.

@jbarton123
Copy link
Contributor Author

Hi Ondre

I see what you mean. I have updated the pull request to thrown an exception if the phar wrapper is not registered.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes and I'll merge it :)

bootstrap.php Outdated
@@ -1,5 +1,9 @@
<?php

if (!in_array('phar', stream_get_wrappers())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add 3rd parameter true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ⚡️

bootstrap.php Outdated
@@ -1,5 +1,9 @@
<?php

if (!in_array('phar', stream_get_wrappers())) {
throw new \Exception('Phar wrapper is not registered. Please review your php.ini settings');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add dot . at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ⚡️

@ondrejmirtes ondrejmirtes merged commit d6754dd into phpstan:master Mar 28, 2019
@ondrejmirtes
Copy link
Member

Thanks a lot!

@jbarton123 jbarton123 deleted the bugfix/Add-check-for-phar-in-stream-wrappers branch March 28, 2019 15:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants