From 701d4e60347d6b0e1391584058dc40bb30ba18aa Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Wed, 25 Sep 2013 09:33:07 +0200
Subject: [PATCH] Issue #1908440 by chx, clemens.tolboom, Cottser,
effulgentsia, heddn, YesCT: Fixed Relax MTimeProtectedFileStorage permissions
for DX, drush integration and world domination.
---
.../Component/PhpStorage/FileStorage.php | 44 +++++++++----
.../MTimeProtectedFastFileStorage.php | 66 ++++---------------
core/lib/Drupal/Core/DrupalKernel.php | 1 +
.../MTimeProtectedFileStorageTest.php | 4 +-
4 files changed, 48 insertions(+), 67 deletions(-)
diff --git a/core/lib/Drupal/Component/PhpStorage/FileStorage.php b/core/lib/Drupal/Component/PhpStorage/FileStorage.php
index d588af4780cc..bdc2621f5178 100644
--- a/core/lib/Drupal/Component/PhpStorage/FileStorage.php
+++ b/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);
diff --git a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php
index 6b7ab5c549ac..25429bb621c3 100644
--- a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php
+++ b/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.
*
diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php
index 1bf9c057c77f..884b27170fd9 100644
--- a/core/lib/Drupal/Core/DrupalKernel.php
+++ b/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.
diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageTest.php
index 355b2480d021..b5caeeb75506 100644
--- a/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageTest.php
+++ b/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.
--
GitLab