diff --git a/core/lib/Drupal/Component/PhpStorage/FileStorage.php b/core/lib/Drupal/Component/PhpStorage/FileStorage.php index d588af4..98e7d88 100644 --- a/core/lib/Drupal/Component/PhpStorage/FileStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/FileStorage.php @@ -55,7 +55,7 @@ public function save($name, $code) { $path = $this->getFullPath($name); $dir = dirname($path); if (!file_exists($dir)) { - mkdir($dir, 0700, TRUE); + mkdir($dir, 0777, TRUE); } return (bool) file_put_contents($path, $code); } @@ -101,25 +101,25 @@ public function deleteAll() { * * @param string $path * A string containing either a file or directory path. + * @param bool $delete_top + * If $path is a directory, then FALSE will only remove the contents of + * the directory, not the directory itself. * * @return boolean * TRUE for success or if path does not exist, FALSE in the event of an * error. */ - protected function unlink($path) { + protected function unlink($path, $delete_top = TRUE) { 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 $delete_top ? @rmdir($path) : TRUE; } return @unlink($path); } diff --git a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php index 6b7ab5c..02b69e5 100644 --- a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php @@ -79,7 +79,7 @@ public function save($name, $data) { if (!@file_put_contents($temporary_path, $data)) { return FALSE; } - chmod($temporary_path, 0400); + 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 @@ -88,7 +88,6 @@ public function save($name, $data) { $directory = $this->getContainingDirectoryFullPath($name); if (file_exists($directory)) { $this->cleanDirectory($directory); - touch($directory); } else { mkdir($directory); @@ -105,7 +104,11 @@ public function save($name, $data) { $i = 0; while (($mtime = $this->getUncachedMTime($directory)) && ($mtime != $previous_mtime)) { $previous_mtime = $mtime; - chmod($directory, 0700); + // If the directory permissions can't be modified then check whether + // it's 0333 already, if not, abort. + if (!@chmod($directory, 0777) && (fileperms($directory) & 0777) != 0333) { + throw new \Exception(sprintf("%s can not be used safely for saving.", $directory)); + } // Reset the file back in the temporary location if this is not the first // iteration. if ($i > 0) { @@ -120,9 +123,9 @@ public function save($name, $data) { 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), + // itself is not writable (set to 0444 at the beginning of this function), // there's no way to tamper with it without access to change permissions. - chmod($directory, 0100); + @chmod($directory, 0333); $i++; } return TRUE; @@ -145,9 +148,8 @@ public function delete($name) { */ protected function ensureDirectory() { if (!file_exists($this->directory)) { - mkdir($this->directory, 0700, TRUE); + mkdir($this->directory, 0777, 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); @@ -158,15 +160,11 @@ protected function ensureDirectory() { * Removes everything in a directory, leaving it empty. * * @param string $directory - * The directory to be emptied out. + * The directory to be emptied out. Note that the directory mtime is + * forced to change. */ protected function cleanDirectory($directory) { - chmod($directory, 0700); - foreach (new \DirectoryIterator($directory) as $fileinfo) { - if (!$fileinfo->isDot()) { - $this->unlink($fileinfo->getPathName()); - } - } + $this->unlink($directory, FALSE); } /** @@ -216,4 +214,20 @@ protected function getUncachedMTime($directory) { return filemtime($directory); } + /** + * Overrides \Drupal\Component\PhpStorage\FileStorage::unlink(). + */ + protected function unlink($path, $delete_top = TRUE) { + try { + return parent::unlink($path,$delete_top); + } + catch (\UnexpectedValueException $e) { + // If the directory has 0333 permissions, we presume it's a holder and + // invalidate it with a touch(). If this doesn't succeed, give up. + if ((fileperms($path) & 0777) == 0333 && @touch($path)) { + return TRUE; + } + throw $e; + } + } } diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 1bf9c05..884b271 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 355b248..5b9b9fb 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, 0333); // Ensure the root directory for the bin has a .htaccess file denying web // access. @@ -84,7 +84,6 @@ function testSecurity() { // a second of the initial save(). sleep(1); for ($i = 0; $i < 2; $i++) { - $storageFactory = new PhpStorageFactory(); $php = $this->storageFactory->get('simpletest'); $GLOBALS['hacked'] = FALSE; $untrusted_code = "