\Drupal\Core\Security\PharExtensionInterceptor is incompatible with GeoIP and other libraries that use phar aliases or Phar::mapPhar()

Thanks for opening this. I'm updated to 7.63 on pantheon, and my local lando dev. Still getting an error.

PHP 7.1.26
Geoip 7.x-2.1

I've double checked my install even though this is a new error. see image.

Here is the full message from my local on lando.

Warning: Uncaught TYPO3\PharStreamWrapper\Exception: Unexpected file extension in "phar://geoip2.phar/vendor/autoload.php" in /app/misc/typo3/drupal-security/PharExtensionInterceptor.php:38
Stack trace:
#0 /app/misc/typo3/phar-stream-wrapper/src/Behavior.php(72): Drupal\Core\Security\PharExtensionInterceptor->assert('phar://geoip2.p...', 'stream_open')
#1 /app/misc/typo3/phar-stream-wrapper/src/Manager.php(83): TYPO3\PharStreamWrapper\Behavior->assert('phar://geoip2.p...', 'stream_open')
#2 /app/misc/typo3/phar-stream-wrapper/src/PharStreamWrapper.php(412): TYPO3\PharStreamWrapper\Manager->assert('phar://geoip2.p...', 'stream_open')
#3 /app/misc/typo3/phar-stream-wrapper/src/PharStreamWrapper.php(247): TYPO3\PharStreamWrapper\PharStreamWrapper->assert('phar://geoip2.p...', 'stream_open')
#4 phar:///app/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php(3): TYPO3\PharStreamWrapper\PharStreamWrapper->stream_open('phar://geoip2.p...', 'rb', 129, NULL)
#5 phar:///app/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar in /app/misc/typo3/drupal-security/PharExtensionInterceptor.php on line 38

Call Stack:
    0.0128     395712   1. {main}() /app/index.php:0
    1.0280    6324360   2. menu_execute_active_handler() /app/index.php:21
    3.6212   12783040   3. drupal_deliver_page() /app/includes/menu.inc:542
    3.6222   12783120   4. drupal_deliver_html_page() /app/includes/common.inc:2634
    3.6226   12784376   5. drupal_render_page() /app/includes/common.inc:2761
    3.6574   12845488   6. eu_cookie_compliance_page_build() /app/includes/common.inc:5931
    3.6591   12848664   7. eu_cookie_compliance_user_in_eu() /app/sites/all/modules/eu_cookie_compliance/eu_cookie_compliance.module:114
    3.6591   12848664   8. geoip_country_code() /app/sites/all/modules/eu_cookie_compliance/eu_cookie_compliance.module:457
    3.6592   12848664   9. geoip_instance() /app/sites/all/modules/geoip/geoip.module:266
    3.6622   12850616  10. Drupal\geoip\GeoIpHandlerV2->__construct() /app/sites/all/modules/geoip/geoip.module:156
    3.6773   12929456  11. libraries_load() /app/sites/all/modules/geoip/src/GeoIpHandlerV2.php:47
    3.6809   12930928  12. libraries_load_files() /app/sites/all/modules/libraries/libraries.module:690
    3.6818   12932344  13. _libraries_require_once() /app/sites/all/modules/libraries/libraries.module:787
    3.6876   12973728  14. require_once('/app/sites/all/libraries/GeoIP2-phar/geoip2.phar') /app/sites/all/modules/libraries/libraries.module:837
    3.6910   12967296  15. require('phar:///app/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php') /app/sites/all/libraries/GeoIP2-phar/geoip2.phar:9


Fatal error: main(): Failed opening required 'phar://geoip2.phar/vendor/autoload.php' (include_path='.:/usr/local/lib/php') in phar:///app/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php on line 3

Call Stack:
    0.0128     395712   1. {main}() /app/index.php:0
    1.0280    6324360   2. menu_execute_active_handler() /app/index.php:21
    3.6212   12783040   3. drupal_deliver_page() /app/includes/menu.inc:542
    3.6222   12783120   4. drupal_deliver_html_page() /app/includes/common.inc:2634
    3.6226   12784376   5. drupal_render_page() /app/includes/common.inc:2761
    3.6574   12845488   6. eu_cookie_compliance_page_build() /app/includes/common.inc:5931
    3.6591   12848664   7. eu_cookie_compliance_user_in_eu() /app/sites/all/modules/eu_cookie_compliance/eu_cookie_compliance.module:114
    3.6591   12848664   8. geoip_country_code() /app/sites/all/modules/eu_cookie_compliance/eu_cookie_compliance.module:457
    3.6592   12848664   9. geoip_instance() /app/sites/all/modules/geoip/geoip.module:266
    3.6622   12850616  10. Drupal\geoip\GeoIpHandlerV2->__construct() /app/sites/all/modules/geoip/geoip.module:156
    3.6773   12929456  11. libraries_load() /app/sites/all/modules/geoip/src/GeoIpHandlerV2.php:47
    3.6809   12930928  12. libraries_load_files() /app/sites/all/modules/libraries/libraries.module:690
    3.6818   12932344  13. _libraries_require_once() /app/sites/all/modules/libraries/libraries.module:787
    3.6876   12973728  14. require_once('/app/sites/all/libraries/GeoIP2-phar/geoip2.phar') /app/sites/all/modules/libraries/libraries.module:837
    3.6910   12967296  15. require('phar:///app/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php') /app/sites/all/libraries/GeoIP2-phar/geoip2.phar:9

Error message from Pantheon

Warning: Uncaught TYPO3\PharStreamWrapper\Exception: Unexpected file extension in "phar://geoip2.phar/vendor/autoload.php" in /srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/misc/typo3/drupal-security/PharExtensionInterceptor.php:38
Stack trace:
#0 /srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/misc/typo3/phar-stream-wrapper/src/Behavior.php(72): Drupal\Core\Security\PharExtensionInterceptor->assert('phar://geoip2.p...', 'stream_open')
#1 /srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/misc/typo3/phar-stream-wrapper/src/Manager.php(83): TYPO3\PharStreamWrapper\Behavior->assert('phar://geoip2.p...', 'stream_open')
#2 /srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/misc/typo3/phar-stream-wrapper/src/PharStreamWrapper.php(412): TYPO3\PharStreamWrapper\Manager->assert('phar://geoip2.p...', 'stream_open')
#3 /srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/misc/typo3/phar-stream-wrapper/src/PharStreamWrapper.php(247): TYPO3\PharStreamWrapper\PharStreamWrapper->assert('phar://geoip2.p...', 'stream_open') in /srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/misc/typo3/drupal-security/PharExtensionInterceptor.php on line 38

Fatal error: main(): Failed opening required 'phar://geoip2.phar/vendor/autoload.php' (include_path='.:/usr/share/pear:/usr/share/php') in phar:///srv/bindings/e82aae45b6bd4ef3987c495e7cf5ce85/code/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php on line 3

This is happening on a couple of my sites with that module.

I can confirm this problem. Latest Drupal core update breaks geoip, if the module is used with geoip2.phar (https://github.com/maxmind/GeoIP2-php/releases).

This results in WSOD on pages, where geoip functionality is in use.

PHP Fatal error: main(): Failed opening required 'phar://geoip2.phar/vendor/autoload.php'

Looking at the stack trace I think \TYPO3\PharStreamWrapper\Helper::determineBaseFile() is working the wrong way around and resolving phar://geoip2.phar/vendor/autoload.php to be a file.

I wonder if the attached patch fixes it. We'll need to push for an upstream fix but at least we can find a resolution first.

Note this patch is a Drupal 8 patch. I'll roll a Drupal 7 patch next.

Here's a D7 patch

@alexpott thanks for providing a patch

I applied #5 on a D7 install, which worked without problems, but the errors still occur.

PHP Warning:  Uncaught TYPO3\\PharStreamWrapper\\Exception: Unexpected file extension in "phar://geoip2.phar/vendor/autoload.php" in .../misc/typo3/drupal-security/PharExtensionInterceptor.php:38\nStack trace:\n#0 /var/www/dev/html/misc/typo3/phar-stream-wrapper/src/Behavior.php(72)...

PHP Fatal error:  main(): Failed opening required 'phar://geoip2.phar/vendor/autoload.php' (include_path='.:/usr/share/php') in phar:///var/www/...sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php on line 3

My current ugly workaround to prevent WSOD is to return TRUE in public function assert without checking anything. That's of course not a solution, it just keeps the site working.

Patch with tests. It turns out the phar has an aliasing system and we need to resolve them. I'm not 100% sure this will work for geoip because I'm not sure how the phar is included but hopefully @indigoxela can test again :)

The patch includes the tests from #3026414: Add tests for PharExtensionInterceptor for invocation from a CLI command and adds the phar alias case.

@alexpott unfortunately 3026443-d7-7.patch doesn't solve the problem either, still WSOD.

I added a "drupal_set_message($path);" in public function assert before returning TRUE.

This is the full output:

phar:///var/www/dev/html/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php
phar://geoip2.phar/vendor/autoload.php
phar://geoip2.phar/vendor/composer/autoload_real.php
phar://geoip2.phar/vendor/composer/ClassLoader.php
phar://geoip2.phar/vendor/composer/autoload_static.php
phar://geoip2.phar/vendor/composer/../../src/Database/Reader.php
phar://geoip2.phar/vendor/composer/../../src/Database/Reader.php
phar://geoip2.phar/vendor/composer/../../src/ProviderInterface.php
phar://geoip2.phar/vendor/composer/../../src/ProviderInterface.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Decoder.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Decoder.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Util.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Util.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Metadata.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Metadata.php
phar://geoip2.phar/vendor/composer/../../src/Model/Country.php
phar://geoip2.phar/vendor/composer/../../src/Model/Country.php
phar://geoip2.phar/vendor/composer/../../src/Model/AbstractModel.php
phar://geoip2.phar/vendor/composer/../../src/Model/AbstractModel.php
phar://geoip2.phar/vendor/composer/../../src/Record/Continent.php
phar://geoip2.phar/vendor/composer/../../src/Record/Continent.php
phar://geoip2.phar/vendor/composer/../../src/Record/AbstractPlaceRecord.php
phar://geoip2.phar/vendor/composer/../../src/Record/AbstractPlaceRecord.php
phar://geoip2.phar/vendor/composer/../../src/Record/AbstractRecord.php
phar://geoip2.phar/vendor/composer/../../src/Record/AbstractRecord.php
phar://geoip2.phar/vendor/composer/../../src/Record/Country.php
phar://geoip2.phar/vendor/composer/../../src/Record/Country.php
phar://geoip2.phar/vendor/composer/../../src/Record/MaxMind.php
phar://geoip2.phar/vendor/composer/../../src/Record/MaxMind.php
phar://geoip2.phar/vendor/composer/../../src/Record/RepresentedCountry.php
phar://geoip2.phar/vendor/composer/../../src/Record/RepresentedCountry.php
phar://geoip2.phar/vendor/composer/../../src/Record/Traits.php
phar://geoip2.phar/vendor/composer/../../src/Record/Traits.php

I think this will work. At least it does for me when using geoip. I have not thought through all of the ramifications yet though. The problem is that the \Drupal\Core\Security\PharExtensionInterceptor cannot possible resolve a path like phar://geoip2.phar/vendor/autoload.php because the alias <code>phar://geoip2.phar only works when used by code contained inside the phar.

@alexpott cool, patch #9 finally solved the problem for Drupal 7

I'm not sure, if this has any downsides for security, so I don't dare yet to set the status to RTBC.
But your comment in code properly points out why this is done.

Here's a test only patch for D8 and fix for D8 too showing the approach works. We just now have to determine whether this opens a security vulnerability.

We;'re going to need to address this in D8 first. Those patches failed because they were tested against D7 :)

+++ b/misc/typo3/drupal-security/PharExtensionInterceptor.php
@@ -57,7 +57,10 @@ class PharExtensionInterceptor implements Assertable {
+      // If we can't determine a base then this is being invoked from inside a
+      // Phar archive that is using aliases. In this case we should allow access
+      // because the real phar will have already been checked.
+      return TRUE;

As discussed in chat, this change is _not secure_ as it opens up a race for placing the file after the assertion succeeded here.

What I would suggest is that aliases need to at least have a .phar extension, so I would do something like:

if ($baseFile === NULL) {
$norm_path = Helper::normalizePath($path);

$baseFile = array_current(explode('/', $path));
$fileExtension = pathinfo($baseFile, PATHINFO_EXTENSION);
return $fileExtension == "phar";

That should work for aliases.

Alex is of course right that aliases do not need to end with .phar, so here is two other ideas:

- The other possibility is to allow to configure a whitelist of aliases.

But maybe we can do even better:

Maybe we could read the alias of a phar file when asserting() it the first time?

There is this for example: http://php.net/manual/en/phar.getalias.php

e.g. So maybe we can do:

$phar = new Phar($baseName);
$alias = $phar->getAlias();
if ($alias) { // no idea to check it against what
  $this->aliases[$alias] = $baseName;
}

We then only need to resolve the alias in case we cannot find a resolvable path.

So here's a fix based on #16 but I think we should move this upstream and maybe into \TYPO3\PharStreamWrapper\PharStreamWrapper

Here's a D7 patch based on the same logic.

+++ b/misc/typo3/drupal-security/PharExtensionInterceptor.php
@@ -57,7 +57,10 @@ class PharExtensionInterceptor implements Assertable {
     if ($baseFile === NULL) {
-      return FALSE;
+      // If we can't determine a base then this is being invoked from inside a
+      // Phar archive that is using aliases. In this case we should allow access
+      // because the real phar will have already been checked.
+      return TRUE;

This seems like a dangerous assumption to make. Can we verify that a real phar archive was checked?

Edit: xpost.

We should also check and see if people who still have Drush issues on 7.63 have this problem: #3026560: After upgrade to 7.63, 8.5.10, 8.6.7, 9.4.0 get TYPO3 phar error for drush

Based on @samuel.mortenson's previous research, I suspect there's something to do with relative paths that's causing their issues and I think this patch might fix those issues as well.

@alexpott unfortunately the D7 patch (3026443-d7-18.patch) doesn't work yet

Still wsod.

Some findings: your strpos check won't work because $alias in my case is "/var/www/dev/html/sites/all/libraries/GeoIP2-phar/geoip2.phar". In other words $alias contains the full filesystem path. You seem to expect the basename.

instead of
strpos($path, 'phar://' . $alias) === 0
try
strpos($path, 'phar://' . basename($alias)) === 0

And there's an additional warning:

Warning: stream_wrapper_register(): class 'Drupal\Core\Security\PharStreamWrapper' is undefined in Drupal\Core\Security\PharExtensionInterceptor->addAliases() (....misc/typo3/drupal-security/PharExtensionInterceptor.php).

@indigoxela - the problem is that

$phar = new \phar('sites/all/libraries/GeoIP2-phar/geoip2.phar');
$phar->getAlias() // This does not return the expected 'geoip2.phar'

I'm guessing this is because of how the phar is built. GeoIP2 uses box - with the following config https://github.com/maxmind/GeoIP2-php/blob/master/box.json

I think box does not call setAlias when building the phar - see https://github.com/box-project/box2/search?q=setAlias&unscoped_q=setAlias - it just uses \Phar::mapPhar() which takes an alias but does not set the global phar alias.

Not. fair.

Well box eventually use https://github.com/box-project/box2-lib/blob/1c5d528d0c44af9d084c2e68ed8... and that creates the phar with new Phar(filename, , alias) so I've no idea why getAlias does not work :(

I've tested

$phar = new \Phar(__DIR__ . '/alias_phar.phar', NULL, 'alias_phar.phar');
var_dump($phar->getAlias());

Add that dumps the expected alias.

Hi everybody,

$phar = new \Phar($compromisedFile);

Basically that's already enough to trigger insecure deserialization (which was the initial aspect of dealing with Phar files in a special way). In order to retrieve the Phar alias, manifest information has to be parsed.

There is an experimental implementation available which is exactly doing this without invoking Phar internals:
https://github.com/TYPO3/phar-stream-wrapper/blob/feature/phar-reader/sr...

In case there is some minima PHP code that would show me the current drawbacks, I'd gladly jump in with giving support on how to solve that and create according functional tests for it.

#26 Thanks - that is a very good catch.

I think there is no other choice than to parse the alias directly.

It seems we can easily do that manually via:

        $content = file_get_contents($baseFile); // Or rather use fseek, fread + fread
        $alias_length = Reader::resolveFourByteLittleEndian($content, 14);
        $alias = substr($content, 18, $alias_length);

Probably we need something like:

  $fp = fopen($baseFile, "rb");
  fseek($fp, 14);
  $len = convert_somehow(fread($fp, 4));
  $alias = fread($fp, $len);

and if we also hardcode an implementation of Reader::resolveFourByteLittleEndian() we don't need to implement all of the experimental reader.

That feels pretty good - as an approach to support the aliases.

It still remains that aliases seems to be implemented in an "interesting" way.

@Fabianx, @oliver.hader we're only calling \Phar on something after we consider it safe - so if that is risky then we have more problems.

Also the latest PR on https://github.com/TYPO3/phar-stream-wrapper/pull/7 moves the alias getting to a better place.

However none of this addresses the problem that the way geoip is built it doesn't have a standard phar alias - it is uses http://php.net/manual/en/phar.mapphar.php

It looks like we're stuck a bit now with path mappings.

Again for clarification:
This (real) path passes the security check:

phar:///var/www/dev/html/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php

These "paths" fail because they are just mappings (or aliases?):

phar://geoip2.phar/vendor/autoload.php
phar://geoip2.phar/vendor/composer/autoload_real.php
phar://geoip2.phar/vendor/composer/ClassLoader.php

See the full path list for geoip2.phar in comment #8

phar:/// vs phar://

And reading this thread, it seems impossible to verify these mappings. Is this still true?
And a simple plausibility check, for instance via regex, won't be secure enough?

A simple example:

  private function baseFileContainsPharExtension($path) {
    $baseFile = Helper::determineBaseFile($path);
    if ($baseFile === NULL) {
      $plausible = preg_match('/^phar:\/\/[a-zA-Z0-9_]+\.phar\/.+\.php$/', $path);
      if ($plausible === 1) {
        return TRUE;
      }
      return FALSE;
    }

What about a different approach: geoip2.phar is known to the libraries module. Can we create something from this? Of course, that would only work for stuff known to be installed libraries, not for drush.

So as stated before, as we always deem .phar files as secure, it's perfectly fine to use the original patch and only return TRUE if the proposed alias has a phar extension.

We also could with some certainty assume that aliases (made with box) have the internal alias be like the basePath -- unless the file was renamed afterwards.

Overall it's a box bug and a misunderstanding in PHP. As stated in GH upstream issue you can "patch" a phar file easily to work with the alias support in the patch here:

$phar = new Phar("geoip2.phar");
$phar->setAlias("geoip2.phar");

with phar.readonly=0 in some php.ini, e.g. a full tool could just do:

#!/bin/bash

file=$1
alias=$2

php -c <(echo "phar.readonly=0") <<EOF
<?php

\$phar = new Phar("$file");
\$phar->setAlias("$alias");
print "Successfully set alias to $alias." . PHP_EOL;
EOF

and invoke with ./patch-alias.sh geoip2.phar geoip2.phar

Unfortunately box is no longer maintained, but adding a simple setAlias call after the new Phar() would solve it all for us.

And a simple plausibility check, for instance via regex, won't be secure enough?

I think it might be. I think allowing /^phar:\/\/[a-zA-Z0-9_]+\.phar\// makes sense as a way of whitelisting aliases that aren't available via other ways. Even if PHP's stream wrapper ends up not resolving it to an actual alias, how would that open up a vulnerability? And by allowing it, it could solve for the geoip case and other simple *.phar alias cases without requiring the burden of #31.

#32That was my original suggestion, too -- but it means that we only support aliases that end in .phar -- which Alex was against.

But any support for aliases is better than none for now -- while we work on getting better support out.

Right. I think we can do #32 in addition to aliases that we discover via getAlias().

I've worked a possible solution that works with GeoIP! And gives use the level of protection we have already. And doesn't read the Phar :)

It works by using the backtrace because it knows that at some point back in the chain the real Phar file must be in $call['file'].

I'd be interested to see if this helps with our issue over in #3026560: After upgrade to 7.63, 8.5.10, 8.6.7, 9.4.0 get TYPO3 phar error for drush since you are using real path to do some of the checking. Might test it against that scenario too.

Promoting to major since this is a contrib blocker.

I haven't tested this, but just from looking at the patch, it looks to me like #36 would be insecure if code within a .phar file does a file operation on an untrusted phar:// URI (a different one than itself). In other words, anywhere the original vulnerability (prior to SA-CORE-2019-002) exists, then with #36, wouldn't that same vulnerability be reintroduced if the code with the original vulnerability happened to get packaged and deployed as a .phar file?

@effulgentsia I don't think so. You can't use a phar's internal alias from another phar file. So the first call to the phar stream wrapper is going to error because $baseFile = Helper::determineBaseFile($path); is going to be the path to your bad phar file and then an exception is going to be thrown.

I guess there might be a worry about a possible timing attack similar to #15? Ie. where you get a good phar to access a bar phar that does not exist yet and somehow is triggered to come into existence after this check and before PHP tries to actually do anything with it. I'm really not sure that is a realistic expectation because

• the timing attack would need to be triggered via the server and the server is not going to be in phar
• the access needs to be direct - i.e. not via any other code because then it would be that code that is analysed in the backtrace and it won't pass the phar extension test

Here are some performance tweaks to do less backtracing.

@alexpott I tested your D7 patch (3026443-36-d7.patch)

It fixes the errors and wsod, but breaks geoip functionality. My geoip rule doesn't work with your patch applied.
Need to dig a little deeper, to figure out, what the problem is.

Ah, got it. Only a part of geoip's paths pass the check now ($this->baseFileContainsPharExtension($path)).

These work:

phar://geoip2.phar/vendor/autoload.php
phar://geoip2.phar/vendor/composer/autoload_real.php
phar://geoip2.phar/vendor/composer/ClassLoader.php
phar://geoip2.phar/vendor/composer/autoload_static.php

These still fail:

phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Decoder.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Metadata.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader.php
phar://geoip2.phar/vendor/composer/../maxmind-db/reader/src/MaxMind/Db/Reader/Util.php
phar://geoip2.phar/vendor/composer/../../src/Database/Reader.php
phar://geoip2.phar/vendor/composer/../../src/Model/AbstractModel.php
phar://geoip2.phar/vendor/composer/../../src/Model/Country.php
phar://geoip2.phar/vendor/composer/../../src/ProviderInterface.php
phar://geoip2.phar/vendor/composer/../../src/Record/AbstractPlaceRecord.php
phar://geoip2.phar/vendor/composer/../../src/Record/AbstractRecord.php
phar://geoip2.phar/vendor/composer/../../src/Record/Continent.php
phar://geoip2.phar/vendor/composer/../../src/Record/Country.php
phar://geoip2.phar/vendor/composer/../../src/Record/MaxMind.php
phar://geoip2.phar/vendor/composer/../../src/Record/RepresentedCountry.php
phar://geoip2.phar/vendor/composer/../../src/Record/Traits.php

It should be obvious, why (/../).

This patch combines the work done by alexpott with a lightweight regex check for a limited set of aliases.

It's for Drupal 7, so of course tests fail with D8.

Note there is a some great stuff going on upstream to read the stub and get the mapped alias from there - see https://github.com/TYPO3/phar-stream-wrapper/pull/12/files which pulls everything together. I'm going to try out the latest changes on our tests when I have a chance.

@alexpott are we supposed to wait for another pull request on github, or are we supposed to test the current master?

@indigoxela there's quite a few things we need to do:

1. Open a PR to backport changes to https://github.com/TYPO3/phar-stream-wrapper/ master to the v2 branch - we need the v2 branch for PHP5 support
2. Get https://github.com/TYPO3/phar-stream-wrapper/pull/12
3. backport https://github.com/TYPO3/phar-stream-wrapper/pull/12 to v2
4. Update our libraries

For D7 we can do a patch to do that all in one step but it should be done with all of those things existing. They don't atm.

Upgrading directly from 8.6.3 to 8.6.7 seems to have worked for me.

Sorry guys, I'm getting the following error too:
2019/01/24 12:17:33 [error] 19214#19214: *111253 FastCGI sent in stderr: "PHP message: PHP Warning: Uncaught TYPO3\PharStreamWrapper\Exception: Unexpected file extension in "phar://geoip2.phar/vendor/autoload.php" in /usr/share/nginx/html/drupal/misc/typo3/drupal-security/PharExtensionInterceptor.php:38
Stack trace:
#0 /usr/share/nginx/html/drupal/misc/typo3/phar-stream-wrapper/src/Behavior.php(72): Drupal\Core\Security\PharExtensionInterceptor->assert('phar://geoip2.p...', 'stream_open')
#1 /usr/share/nginx/html/drupal/misc/typo3/phar-stream-wrapper/src/Manager.php(83): TYPO3\PharStreamWrapper\Behavior->assert('phar://geoip2.p...', 'stream_open')
#2 /usr/share/nginx/html/drupal/misc/typo3/phar-stream-wrapper/src/PharStreamWrapper.php(412): TYPO3\PharStreamWrapper\Manager->assert('phar://geoip2.p...', 'stream_open')
#3 /usr/share/nginx/html/drupal/misc/typo3/phar-stream-wrapper/src/PharStreamWrapper.php(247): TYPO3\PharStreamWrapper\PharStreamWrapper->assert('phar://geoip2.p...', 'stream_open')
#4 phar:///usr/share/nginx/html/drupal/sites/all/libraries/GeoIP2-phar/geoip2.phar/...
PHP message: PHP Fatal error: main(): Failed opening required 'phar://geoip2.phar/vendor/autoload.php' (include_path='.:/usr/share/php') in phar:///usr/share/nginx/html/drupal/sites/all/libraries/GeoIP2-phar/geoip2.phar/phar-stub.php on line 3" while reading response header from upstream...

and getting WSOD after upgrading from Drupal 7.61 to 7.63. Php 7.0, MariaDB

I didn't understand at the end if there's a solution and eventually which patch to apply...

@kinmen work is in progress upstream (on github), but there's currently no patch yet.

As a workaround to fix your broken site, you could edit misc/typo3/drupal-security/PharExtensionInterceptor.php directly.

Open it with a text editor and in line 35 within public function assert add:

    if (preg_match('/^phar:\/\/[a-zA-Z0-9_]+\.phar\/.+\.php$/', $path) === 1) {
      // Lightweight way to whitelist phar aliases, if they contain ".phar" extension
      return TRUE;
    }

right before if ($this->baseFileContainsPharExtension($path)), which causes the errors.

This will not be the final solution, but is only a quick fix for broken sites.

Ufff... thank you so much, your solution has saved my life. I look forward to see a patch, but in the meanwhile the web site is working. Thank you thank you thank you

@indigoxela: could you open a child issue with a patch for just #51? I think it'll help others who are using GeoIP in the meantime. Also, if the upstream solution isn't ready in time for the next bug-fix releases of 8.6.x and 7.x, I think we should consider committing the temporary regex workaround in time for those releases, and a child issue would be a good place to discuss that.

Child issue has been created with a patch for Drupal 7 and geoip2.phar.

We probably should consider alexpott's patch #36, as that could fix problems for other libraries.

https://github.com/TYPO3/phar-stream-wrapper/pull/14 is PR to backport functionality we need from the master branch to v2 as that supports PHP5.3^.

Would just supporting PHP 5.6 and above be too radical?

@prlw1: How would that help with this issue?

I think, that after reading Pull Request #12 that @alexpott references in #46 and #48, that would solve #3027089: "Warning: stat failed for phar:///…/sites/all/modules/…" after upgrade to 7.63 for Drupal 7 and Drupal 8, but it's a little difficult to understand the pull request description. It mentions fixing "includes" and the upstream code refers to using realpath on an unprefixed file path. If that's the case, then we could probably resolve #3027089 as a duplicate of this issue for Drupal 8 and whatever follow-up issue is created for Drupal 7.

However I am still not clear from reading the code in Pull request #12 that it resolves that issue. I would greatly appreciate if anyone could help interpret that code to confirm it would resolve #3027089.

I added the "Needs issue summary update" tag because the issue summary mentions finding steps to replicate, but we already have a patch that Needs work and security review.

In the meantime another bugfix release is out for Drupal 8.

People who needed to patch their D8 sites because of a problem with .phar files have to do it again after updating.

Is there anything we can do to speed up progress with PharExtensionInterceptor?

@indigoxela
might I suggest using composer to patch your site instead of manually doing it? It's a great way to document patches as well as reapply them with each upgrade. Take a look at https://github.com/cweagans/composer-patches... Example my patches look something like this

    "patches": {
            "drupal/memcache": {
                "Undefined method error on Statistics page": "https://www.drupal.org/files/issues/2018-09-05/memcache_undefined-2997537-3.patch"
            },
            "drupal/core": {
                "Verify that the configured service exists before calling it in CacheFactory": "https://www.drupal.org/files/issues/2018-03-11/2429321-cache-factory-service-check-38.patch",
                "Dynamic Views Arguments via Tokens": "https://www.drupal.org/files/issues/entity_autocomplete_1699378_156.patch",
                "Text (plain, long) field does not appear in combine fields filter": "https://www.drupal.org/files/issues/2018-11-09/2695207-31.patch"
            },
            "drupal/slick": {
                "Using bower or composer to download the slick library is cumbersome.": "https://www.drupal.org/files/issues/2855190-13_0.patch"
            },
            "drupal/stage_file_proxy": {
                "Drush command doesn't work with verify option": "https://www.drupal.org/files/issues/verify_option_drush_command-2921750.patch"
            },
            "drupal/disqus": {
                "Disqus migrate support is broken": "https://www.drupal.org/files/issues/disqus-migrate-support-2879592-14.patch"
            },
            "drupal/views_ef_fieldset": {
                "Corrects notice on views that do not use exposed filters": "https://www.drupal.org/files/issues/views_ef_fieldset-n2902402-2.patch"
            },
            "drupal/inline_entity_form": {
                "Widget settings to control removing & deleting existing references": "https://www.drupal.org/files/issues/2018-11-23/ief_allow-remove-setting--2875716-11.patch"
            },
            "drupal/entity_browser": {
                "Contextual filter arguments in view displays no entities in the browser": "https://www.drupal.org/files/issues/2018-11-16/contextual-filters-for-view-widget-2790951-23_0.patch"
            }
        },

Keep in mind you can point to a path as well... such as a "patches" folder at the root of your project. with something like

    "patches": {
        "drupal/core": {
            "my custom patch": "path/relative/to/composer.json/my_custom_patch.patch"
        }
    }

might I suggest using composer to patch your site instead of manually doing it

You may suggest anything. ;) Yes, composer can be very handy.

But the actual intention of my comment was to warn people affected by this regression, that they shouldn't expect this issue to be fixed in current releases.

And I hoped for some progress with PharExtensionInterceptor.

So https://github.com/TYPO3/phar-stream-wrapper/pull/14 is merged. We're on the plan from #48

1. Open a PR to backport changes to https://github.com/TYPO3/phar-stream-wrapper/ master to the v2 branch - we need the v2 branch for PHP5 support
2. Get https://github.com/TYPO3/phar-stream-wrapper/pull/12
3. backport https://github.com/TYPO3/phar-stream-wrapper/pull/12 to v2
4. Update our libraries

I'll open a PR to for step 3 later today.

@oliver.hader opened the backport PR and did the work - yay! See https://github.com/TYPO3/phar-stream-wrapper/pull/15

It's not just .phar files that seem to be broken... I use the following code to expand an archive into a path:

$phar = new PharData($archive_path);
$phar->extractTo($temp_dir);

And that is now hitting an error that breaks functionality:

Unexpected file extension in "phar:///var/www/html/docroot/sites/default/files/project-starter-templates/php-template.tar.gz/"

It seems like the initiative to start using PharData() in core for archive handling would also be stymied by this security fix: #2891871: Rewrite the Archiver classes to use PharData. (The patch from earlier in this issue does not fix the problem either.)

It seems like PR 15 has been merged into v2.

And a Joomla user has tested it successfully with geoip2.phar.

What are our next steps? What has to be done on Drupal side now?

We have to bump our composer to use the dev master and try implementing with Drupal. Once we've got a successful integration we need to ask for a release of the phar wrapper and use that.

I updated the issue summary with the current work and tasks that was initially outlined in #48 and based on recent work in #62 and #65.

We use issue tags to help identify next steps, @indigoxela. On any issue, you can now look at the side bar that lists the tags and an "about tags" link. Clicking that will help describe some of the tasks associated with the tag. In this case, I think that the issue summary itself is confusing what the next steps are.

It would help to update the issue summary to include comments about how to reproduce and what libraries and contrib. modules are affected. After updating an issue summary, we remove the tag when we think we're done.

I think the next steps are done now that SA-Core-2019-007 is out. All the PRs were merged into v2 and a new release was made for both typo3-stream-wrapper and Drupal.

I think this can be closed as outdated because that was the only code change, and there wasn't anything planned for \Drupal\Core\Security\PharExtensionInterceptor.

We're very nearly there! We need to update \Drupal\Core\Security\PharExtensionInterceptor - unfortunately we can't just use Typo3's \TYPO3\PharStreamWrapper\Interceptor\PharExtensionInterceptor because it break when used with a renamed Drush phar file.

So there is still something to do here - we just need to work out what.

Here's a patch for Drupal 8 - you can make the same changes to the PharExtensionInterceptor on Drupal 7 and it'll work.

Here's an updated patch for D7, based on the patch in #71.

Concerning patch #72 for D7 I can confirm that it fixes the problem with GeoIP2-phar. No more exceptions and GeoIP works correctly.

Hi all,
I am getting "TYPO3\PharStreamWrapper\Exception: Unexpected file extension in "phar://httpful.phar/Httpful/Bootstrap.php" in Drupal\Core\Security\PharExtensionInterceptor->assert() (line 38 of /Applications/MAMP/htdocs/grammy/docroot/misc/typo3/drupal-security/PharExtensionInterceptor.php)." error for using HTTPFUL phar file.
I ran drush updatedb to confirm if any updates were pending and there weren't any. For me drush cc all command is also working fine.
I just face this issue when I use "include('httpful.phar');" by following the steps provided in http://phphttpclient.com/ Option 1.
Could someone let me know if I missed anything here and how can I get this resolved.
Thanks

Hi,
Just an update. #72 patch worked for me. Thanks @jstoller.

Re-rolled.

Works for me

Patch #77 works fine for me as well
RTBC + 1

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

\Drupal\Core\Security\PharExtensionInterceptor was removed from D10 in https://www.drupal.org/project/drupal/issues/3210486, so this issue no longer applies to Drupal core.

Thanks @akalata! Saving credits for those who worked on this according to our issue credit guidelines, including our upstream colleagues at TYPO3.