Issue #1908440 by chx, clemens.tolboom, Cottser, effulgentsia, heddn, YesCT:...

Issue #1908440 by chx, clemens.tolboom, Cottser, effulgentsia, heddn, YesCT: Fixed Relax MTimeProtectedFileStorage permissions for DX, drush integration and world domination.

core/lib/Drupal/Component/PhpStorage/FileStorage.php

@@ -53,13 +53,38 @@ public function load($name) {
    */
   public function save($name, $code) {
     $path = $this->getFullPath($name);
-    $dir = dirname($path);
-    if (!file_exists($dir)) {
-      mkdir($dir, 0700, TRUE);
-    }
+    $this->ensureDirectory(dirname($path));
     return (bool) file_put_contents($path, $code);
   }
 
+  /**
+   * Ensures the root directory exists and has the right permissions.
+   *
+   * @param string $directory
+   *   The directory path.
+   *
+   * @param int $mode
+   *   The mode, permissions, the directory should have.
+   */
+  protected function ensureDirectory($directory, $mode = 0777) {
+    if (!file_exists($directory)) {
+      // mkdir() obeys umask() so we need to mkdir() and chmod() manually.
+      $parts = explode('/', $directory);
+      $path = '';
+      $delimiter = '';
+      do {
+        $part = array_shift($parts);
+        $path .= $delimiter . $part;
+        $delimiter = '/';
+        // For absolute paths the first part will be empty.
+        if ($part && !file_exists($path)) {
+          mkdir($path);
+          chmod($path, $mode);
+        }
+      } while ($parts);
+    }
+  }
+
   /**
    * Implements Drupal\Component\PhpStorage\PhpStorageInterface::delete().
    */
@@ -109,16 +134,13 @@ public function deleteAll() {
   protected function unlink($path) {
     if (file_exists($path)) {
       // Ensure the file / folder is writable.
-      chmod($path, 0700);
       if (is_dir($path)) {
-        $dir = dir($path);
-        while (($entry = $dir->read()) !== FALSE) {
-          if ($entry == '.' || $entry == '..') {
-            continue;
+        @chmod($path, 0777);
+        foreach (new \DirectoryIterator($path) as $fileinfo) {
+          if (!$fileinfo->isDot()) {
+            $this->unlink($fileinfo->getPathName());
           }
-          $this->unlink($path . '/' . $entry);
         }
-        $dir->close();
         return @rmdir($path);
       }
       return @unlink($path);

core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php

@@ -41,6 +41,8 @@ class MTimeProtectedFastFileStorage extends FileStorage {
 
   /**
    * The .htaccess code to make a directory private.
+   *
+   * Disabling Options Indexes is particularly important.
    */
   const HTACCESS="SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006\nDeny from all\nOptions None\nOptions +FollowSymLinks";
 
@@ -71,7 +73,11 @@ public function __construct(array $configuration) {
    * Implements Drupal\Component\PhpStorage\PhpStorageInterface::save().
    */
   public function save($name, $data) {
-    $this->ensureDirectory();
+    $this->ensureDirectory($this->directory);
+    $htaccess_path =  $this->directory . '/.htaccess';
+    if (!file_exists($htaccess_path) && file_put_contents($htaccess_path, self::HTACCESS)) {
+      @chmod($htaccess_path, 0444);
+    }
 
     // Write the file out to a temporary location. Prepend with a '.' to keep it
     // hidden from listings and web servers.
@@ -79,7 +85,9 @@ public function save($name, $data) {
     if (!@file_put_contents($temporary_path, $data)) {
       return FALSE;
     }
-    chmod($temporary_path, 0400);
+    // The file will not be chmod() in the future so this is the final
+    // permission.
+    chmod($temporary_path, 0444);
 
     // Prepare a directory dedicated for just this file. Ensure it has a current
     // mtime so that when the file (hashed on that mtime) is moved into it, the
@@ -87,12 +95,9 @@ public function save($name, $data) {
     // the rename, in which case we'll try again).
     $directory = $this->getContainingDirectoryFullPath($name);
     if (file_exists($directory)) {
-      $this->cleanDirectory($directory);
-      touch($directory);
-    }
-    else {
-      mkdir($directory);
+      $this->unlink($directory);
     }
+    $this->ensureDirectory($directory);
 
     // Move the file to its final place. The mtime of a directory is the time of
     // the last file create or delete in the directory. So the moving will
@@ -105,7 +110,6 @@ public function save($name, $data) {
     $i = 0;
     while (($mtime = $this->getUncachedMTime($directory)) && ($mtime != $previous_mtime)) {
       $previous_mtime = $mtime;
-      chmod($directory, 0700);
       // Reset the file back in the temporary location if this is not the first
       // iteration.
       if ($i > 0) {
@@ -118,57 +122,11 @@ public function save($name, $data) {
       }
       $full_path = $this->getFullPath($name, $directory, $mtime);
       rename($temporary_path, $full_path);
-
-      // Leave the directory neither readable nor writable. Since the file
-      // itself is not writable (set to 0400 at the beginning of this function),
-      // there's no way to tamper with it without access to change permissions.
-      chmod($directory, 0100);
       $i++;
     }
     return TRUE;
   }
 
-  /**
-   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::delete().
-   */
-  public function delete($name) {
-    $directory = dirname($this->getFullPath($name));
-    if (file_exists($directory)) {
-      $this->cleanDirectory($directory);
-      return rmdir($directory);
-    }
-    return FALSE;
-  }
-
-  /**
-   * Ensures the root directory exists and has correct permissions.
-   */
-  protected function ensureDirectory() {
-    if (!file_exists($this->directory)) {
-      mkdir($this->directory, 0700, TRUE);
-    }
-    chmod($this->directory, 0700);
-    $htaccess_path =  $this->directory . '/.htaccess';
-    if (!file_exists($htaccess_path) && file_put_contents($htaccess_path, self::HTACCESS)) {
-      @chmod($htaccess_path, 0444);
-    }
-  }
-
-  /**
-   * Removes everything in a directory, leaving it empty.
-   *
-   * @param string $directory
-   *   The directory to be emptied out.
-   */
-  protected function cleanDirectory($directory) {
-    chmod($directory, 0700);
-    foreach (new \DirectoryIterator($directory) as $fileinfo) {
-      if (!$fileinfo->isDot()) {
-        $this->unlink($fileinfo->getPathName());
-      }
-    }
-  }
-
   /**
    * Returns the full path where the file is or should be stored.
    *

core/lib/Drupal/Core/DrupalKernel.php

@@ -366,6 +366,7 @@ protected function getKernelParameters() {
    * Initializes the service container.
    */
   protected function initializeContainer() {
+    $this->containerNeedsDumping = FALSE;
     $persist = $this->getServicesToPersist();
     // If we are rebuilding the kernel and we are in a request scope, store
     // request info so we can add them back after the rebuild.

core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageTest.php

@@ -71,8 +71,8 @@ function testSecurity() {
     // minimal permissions. fileperms() can return high bits unrelated to
     // permissions, so mask with 0777.
     $this->assertTrue(file_exists($expected_filename));
-    $this->assertSame(fileperms($expected_filename) & 0777, 0400);
-    $this->assertSame(fileperms($expected_directory) & 0777, 0100);
+    $this->assertSame(fileperms($expected_filename) & 0777, 0444);
+    $this->assertSame(fileperms($expected_directory) & 0777, 0777);
 
     // Ensure the root directory for the bin has a .htaccess file denying web
     // access.