SA-CORE-2019-002 by greggles, cashwilliams, EclipseGc, larowlan, samuel.mortenson, alexpott, tedbow, effulgentsia, Fabianx, xjm, mlhess

Author: xjm

composer.json

@@ -31,6 +31,7 @@
         "symfony/process": "~3.4.0",
         "symfony/polyfill-iconv": "^1.0",
         "symfony/yaml": "~3.4.5",
+        "typo3/phar-stream-wrapper": "^2.0.1",
         "twig/twig": "^1.35.0",
         "doctrine/common": "^2.5",
         "doctrine/annotations": "^1.2",

lib/Drupal/Core/DrupalKernel.php

@@ -19,6 +19,7 @@
 use Drupal\Core\Http\TrustedHostsRequestFactory;
 use Drupal\Core\Installer\InstallerRedirectTrait;
 use Drupal\Core\Language\Language;
+use Drupal\Core\Security\PharExtensionInterceptor;
 use Drupal\Core\Security\RequestSanitizer;
 use Drupal\Core\Site\Settings;
 use Drupal\Core\Test\TestDatabase;
@@ -35,6 +36,9 @@
 use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 use Symfony\Component\HttpKernel\TerminableInterface;
 use Symfony\Component\Routing\Route;
+use TYPO3\PharStreamWrapper\Manager as PharStreamWrapperManager;
+use TYPO3\PharStreamWrapper\Behavior as PharStreamWrapperBehavior;
+use TYPO3\PharStreamWrapper\PharStreamWrapper;
 
 /**
  * The DrupalKernel class is the core of Drupal itself.
@@ -471,6 +475,26 @@ public function boot() {
     // Initialize the container.
     $this->initializeContainer();
 
+    if (in_array('phar', stream_get_wrappers(), TRUE)) {
+      // Set up a stream wrapper to handle insecurities due to PHP's builtin
+      // phar stream wrapper. This is not registered as a regular stream wrapper
+      // to prevent \Drupal\Core\File\FileSystem::validScheme() treating "phar"
+      // as a valid scheme.
+      try {
+        $behavior = new PharStreamWrapperBehavior();
+        PharStreamWrapperManager::initialize(
+          $behavior->withAssertion(new PharExtensionInterceptor())
+        );
+      }
+      catch (\LogicException $e) {
+        // Continue if the PharStreamWrapperManager is already initialized. For
+        // example, this occurs during a module install.
+        // @see \Drupal\Core\Extension\ModuleInstaller::install()
+      };
+      stream_wrapper_unregister('phar');
+      stream_wrapper_register('phar', PharStreamWrapper::class);
+    }
+
     $this->booted = TRUE;
 
     return $this;

lib/Drupal/Core/Security/PharExtensionInterceptor.php

@@ -0,0 +1,73 @@
+<?php
+
+namespace Drupal\Core\Security;
+
+use TYPO3\PharStreamWrapper\Assertable;
+use TYPO3\PharStreamWrapper\Helper;
+use TYPO3\PharStreamWrapper\Exception;
+
+/**
+ * An alternate PharExtensionInterceptor to support phar-based CLI tools.
+ *
+ * @see \TYPO3\PharStreamWrapper\Interceptor\PharExtensionInterceptor
+ */
+class PharExtensionInterceptor implements Assertable {
+
+  /**
+   * Determines whether phar file is allowed to execute.
+   *
+   * The phar file is allowed to execute if:
+   * - the base file name has a ".phar" suffix.
+   * - it is the CLI tool that has invoked the interceptor.
+   *
+   * @param string $path
+   *   The path of the phar file to check.
+   *
+   * @param string $command
+   *   The command being carried out.
+   *
+   * @return bool
+   *   TRUE if the phar file is allowed to execute.
+   *
+   * @throws Exception
+   *   Thrown when the file is not allowed to execute.
+   */
+  public function assert($path, $command) {
+    if ($this->baseFileContainsPharExtension($path)) {
+      return TRUE;
+    }
+    throw new Exception(
+      sprintf(
+        'Unexpected file extension in "%s"',
+        $path
+      ),
+      1535198703
+    );
+  }
+
+  /**
+   * @param string $path
+   *   The path of the phar file to check.
+   *
+   * @return bool
+   *   TRUE if the file has a .phar extension or if the execution has been
+   *   invoked by the phar file.
+   */
+  private function baseFileContainsPharExtension($path) {
+    $baseFile = Helper::determineBaseFile($path);
+    if ($baseFile === NULL) {
+      return FALSE;
+    }
+    // If the stream wrapper is registered by invoking a phar file that does
+    // not not have .phar extension then this should be allowed. For
+    // example, some CLI tools recommend removing the extension.
+    $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
+    $caller = array_pop($backtrace);
+    if (isset($caller['file']) && $baseFile === $caller['file']) {
+      return TRUE;
+    }
+    $fileExtension = pathinfo($baseFile, PATHINFO_EXTENSION);
+    return strtolower($fileExtension) === 'phar';
+  }
+
+}

modules/file/file.module

@@ -23,7 +23,7 @@ use Drupal\Core\Template\Attribute;
 /**
  * The regex pattern used when checking for insecure file types.
  */
-define('FILE_INSECURE_EXTENSION_REGEX', '/\.(php|pl|py|cgi|asp|js)(\.|$)/i');
+define('FILE_INSECURE_EXTENSION_REGEX', '/\.(phar|php|pl|py|cgi|asp|js)(\.|$)/i');
 
 // Load all Field module hooks for File.
 require_once __DIR__ . '/file.field.inc';

modules/simpletest/files/phar-1.phar

@@ -0,0 +1,33 @@
+<?php
+
+namespace Drupal\KernelTests\Core\File;
+
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests that the phar stream wrapper works.
+ *
+ * @group File
+ */
+class PharWrapperTest extends KernelTestBase {
+
+  /**
+   * Tests that only valid phar files can be used.
+   */
+  public function testPharFile() {
+    $base = $this->getDrupalRoot() . '/core/modules/simpletest/files';
+    // Ensure that file operations via the phar:// stream wrapper work for phar
+    // files with the .phar extension.
+    $this->assertFalse(file_exists("phar://$base/phar-1.phar/no-such-file.php"));
+    $this->assertTrue(file_exists("phar://$base/phar-1.phar/index.php"));
+    $file_contents = file_get_contents("phar://$base/phar-1.phar/index.php");
+    $expected_hash = 'c7e7904ea573c5ebea3ef00bb08c1f86af1a45961fbfbeb1892ff4a98fd73ad5';
+    $this->assertSame($expected_hash, hash('sha256', $file_contents));
+
+    // Ensure that file operations via the phar:// stream wrapper throw an
+    // exception for files without the .phar extension.
+    $this->setExpectedException('TYPO3\PharStreamWrapper\Exception');
+    file_exists("phar://$base/image-2.jpg/index.php");
+  }
+
+}

tests/Drupal/KernelTests/Core/File/PharWrapperTest.php

@@ -144,4 +144,31 @@ public function testGetValidStreamScheme() {
     $this->assertFalse(file_stream_wrapper_valid_scheme(file_uri_scheme('foo://asdf')), 'Did not get a valid stream scheme from foo://asdf');
   }
 
+  /**
+   * Tests that phar stream wrapper is registered as expected.
+   *
+   * @see \Drupal\Core\StreamWrapper\StreamWrapperManager::register()
+   */
+  public function testPharStreamWrapperRegistration() {
+    if (!in_array('phar', stream_get_wrappers(), TRUE)) {
+      $this->markTestSkipped('There is no phar stream wrapper registered. PHP is probably compiled without phar support.');
+    }
+    // Ensure that phar is not treated as a valid scheme.
+    $stream_wrapper_manager = $this->container->get('stream_wrapper_manager');
+    $this->assertFalse($stream_wrapper_manager->getViaScheme('phar'));
+
+    // Ensure that calling register again and unregister do not create errors
+    // due to the PharStreamWrapperManager singleton.
+    $stream_wrapper_manager->register();
+    $this->assertContains('public', stream_get_wrappers());
+    $this->assertContains('phar', stream_get_wrappers());
+    $stream_wrapper_manager->unregister();
+    $this->assertNotContains('public', stream_get_wrappers());
+    // This will have reverted to the builtin phar stream wrapper.
+    $this->assertContains('phar', stream_get_wrappers());
+    $stream_wrapper_manager->register();
+    $this->assertContains('public', stream_get_wrappers());
+    $this->assertContains('phar', stream_get_wrappers());
+  }
+
 }