Issue #1675260 by chx, effulgentsia, pwolanin, neclimdul: Implement PHP...

Issue #1675260 by chx, effulgentsia, pwolanin, neclimdul: Implement PHP reading/writing secured against 'leaky' script.

core/includes/bootstrap.inc

@@ -3538,3 +3538,51 @@ function drupal_check_memory_limit($required, $memory_limit = NULL) {
   //   the operation.
   return ((!$memory_limit) || ($memory_limit == -1) || (parse_size($memory_limit) >= parse_size($required)));
 }
+
+/**
+ * Instantiates and statically caches a storage controller for generated PHP code.
+ *
+ * By default, this returns an instance of the
+ * Drupal\Component\PhpStorage\MTimeProtectedFileStorage class.
+ *
+ * Classes implementing
+ * Drupal\Component\PhpStorage\PhpStorageInterface can be registered for a
+ * specific bin or as a default implementation.
+ *
+ * @param $name
+ *   The name for which the storage controller should be returned. Defaults to
+ *   'default'. The name is also used as the storage bin if one is not
+ *   specified in the configuration.
+ *
+ * @return Drupal\Component\PhpStorage\PhpStorageInterface
+ *   An instantiated storage controller for the specified name.
+ *
+ * @see Drupal\Component\PhpStorage\PhpStorageInterface
+ */
+function drupal_php_storage($name = 'default') {
+  global $conf;
+  $storage_controllers = &drupal_static(__FUNCTION__);
+  if (!isset($storage_controllers[$name])) {
+    if (isset($conf['php_storage'][$name])) {
+      $configuration = $conf['php_storage'][$name];
+    }
+    elseif (isset($conf['php_storage']['default'])) {
+      $configuration = $conf['php_storage']['default'];
+    }
+    else {
+      $configuration = array(
+        'class' => 'Drupal\Component\PhpStorage\MTimeProtectedFileStorage',
+        'secret' => $GLOBALS['drupal_hash_salt'],
+      );
+    }
+    $class = isset($configuration['class']) ? $configuration['class'] : 'Drupal\Component\PhpStorage\MTimeProtectedFileStorage';
+    if (!isset($configuration['bin'])) {
+      $configuration['bin'] = $name;
+    }
+    if (!isset($configuration['directory'])) {
+      $configuration['directory'] = DRUPAL_ROOT . '/' . variable_get('file_public_path', conf_path() . '/files') . '/php';
+    }
+    $storage_controllers[$name] = new $class($configuration);
+  }
+  return $storage_controllers[$name];
+}

core/includes/file.inc

@@ -1317,6 +1317,10 @@ function file_unmanaged_delete($path) {
  *
  * @param $path
  *   A string containing either an URI or a file or directory path.
+ * @param $callback
+ *   (optional) Callback function to run on each file prior to deleting it and
+ *   on each directory prior to traversing it. For example, can be used to
+ *   modify permissions.
  *
  * @return
  *   TRUE for success or if path does not exist, FALSE in the event of an
@@ -1324,7 +1328,10 @@ function file_unmanaged_delete($path) {
  *
  * @see file_unmanaged_delete()
  */
-function file_unmanaged_delete_recursive($path) {
+function file_unmanaged_delete_recursive($path, $callback = NULL) {
+  if (isset($callback)) {
+    call_user_func($callback, $path);
+  }
   if (is_dir($path)) {
     $dir = dir($path);
     while (($entry = $dir->read()) !== FALSE) {
@@ -1332,7 +1339,7 @@ function file_unmanaged_delete_recursive($path) {
         continue;
       }
       $entry_path = $path . '/' . $entry;
-      file_unmanaged_delete_recursive($entry_path);
+      file_unmanaged_delete_recursive($entry_path, $callback);
     }
     $dir->close();
 

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

+<?php
+
+/**
+ * @file
+ * Definition of Drupal\Component\PhpStorage\FileStorage.
+ */
+
+namespace Drupal\Component\PhpStorage;
+
+/**
+ * Reads code as regular PHP files, but won't write them.
+ */
+class FileReadOnlyStorage implements PhpStorageInterface {
+
+  /**
+   * The directory where the files should be stored.
+   *
+   * @var string
+   */
+  protected $directory;
+
+  /**
+   * Constructs this FileStorage object.
+   *
+   * @param $configuration
+   *   An associative array, containing at least two keys (the rest are ignored):
+   *   - directory: The directory where the files should be stored.
+   *   - bin: The storage bin. Multiple storage objects can be instantiated with
+   *   the same configuration, but for different bins.
+   */
+  public function __construct(array $configuration) {
+
+    $this->directory = $configuration['directory'] . '/' . $configuration['bin'];
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists().
+   */
+  public function exists($name) {
+    return file_exists($this->getFullPath($name));
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load().
+   */
+  public function load($name) {
+    // The FALSE returned on failure is enough for the caller to handle this,
+    // we do not want a warning too.
+    return (@include_once $this->getFullPath($name)) !== FALSE;
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::save().
+   */
+  public function save($name, $code) {
+    return FALSE;
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::delete().
+   */
+  public function delete($name) {
+    return FALSE;
+  }
+
+  /**
+   * Returns the full path where the file is or should be stored.
+   */
+  protected function getFullPath($name) {
+    return $this->directory . '/' . $name;
+  }
+}

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

+<?php
+
+/**
+ * @file
+ * Definition of Drupal\Component\PhpStorage\FileStorage.
+ */
+
+namespace Drupal\Component\PhpStorage;
+
+/**
+ * Stores the code as regular PHP files.
+ */
+class FileStorage implements PhpStorageInterface {
+
+  /**
+   * The directory where the files should be stored.
+   *
+   * @var string
+   */
+  protected $directory;
+
+  /**
+   * Constructs this FileStorage object.
+   *
+   * @param $configuration
+   *   An associative array, containing at least these two keys:
+   *   - directory: The directory where the files should be stored.
+   *   - bin: The storage bin. Multiple storage objects can be instantiated with the
+   *     same configuration, but for different bins..
+   */
+  public function __construct(array $configuration) {
+    $this->directory = $configuration['directory'] . '/' . $configuration['bin'];
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists().
+   */
+  public function exists($name) {
+    return file_exists($this->getFullPath($name));
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load().
+   */
+  public function load($name) {
+    // The FALSE returned on failure is enough for the caller to handle this,
+    // we do not want a warning too.
+    return (@include_once $this->getFullPath($name)) !== FALSE;
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::save().
+   */
+  public function save($name, $code) {
+    $path = $this->getFullPath($name);
+    mkdir(dirname($path), 0700, TRUE);
+    return (bool) file_put_contents($path, $code);
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::delete().
+   */
+  public function delete($name) {
+    $path = $this->getFullPath($name);
+    return @unlink($path);
+  }
+
+  /**
+   * Returns the full path where the file is or should be stored.
+   */
+  protected function getFullPath($name) {
+    return $this->directory . '/' . $name;
+  }
+}

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

+<?php
+
+/**
+ * @file
+ * Definition of Drupal\Component\PhpStorage\MTimeProtectedFastFileStorage.
+ */
+namespace Drupal\Component\PhpStorage;
+
+use DirectoryIterator;
+
+/**
+ * Stores PHP code in files with securely hashed names.
+ *
+ * The goal of this class is to ensure that if a PHP file is replaced with
+ * an untrusted one, it does not get loaded. Since mtime granularity is 1
+ * second, we cannot prevent an attack that happens within one second of the
+ * initial save(). However, it is very unlikely for an attacker exploiting an
+ * upload or file write vulnerability to also know when a legitimate file is
+ * being saved, discover its hash, undo its file permissions, and override the
+ * file with an upload all within a single second. Being able to accomplish
+ * that would indicate a site very likely vulnerable to many other attack
+ * vectors.
+ *
+ * Each file is stored in its own unique containing directory. The hash is based
+ * on the virtual file name, the containing directory's mtime, and a
+ * cryptographically hard to guess secret string. Thus, even if the hashed file
+ * name is discovered and replaced by an untrusted file (e.g., via a
+ * move_uploaded_file() invocation by a script that performs insufficient
+ * validation), the directory's mtime gets updated in the process, invalidating
+ * the hash and preventing the untrusted file from getting loaded.
+ *
+ * This class does not protect against overwriting a file in-place (e.g. a
+ * malicious module that does a file_put_contents()) since this will not change
+ * the mtime of the directory. MTimeProtectedFileStorage protects against this
+ * at the cost of an additional system call for every load() and exists().
+ *
+ * The containing directory is created with the same name as the virtual file
+ * name (slashes removed) to assist with debugging, since the file itself is
+ * stored with a name that's meaningless to humans.
+ */
+class MTimeProtectedFastFileStorage extends FileStorage {
+
+  /**
+   * The secret used in the HMAC.
+   *
+   * @var string
+   */
+  protected $secret;
+
+  /**
+   * Constructs this MTimeProtectedFastFileStorage object.
+   *
+   * @param $configuration
+   *   An associated array, containing at least these keys (the rest are
+   *   ignored):
+   *   - directory: The directory where the files should be stored.
+   *   - secret: A cryptographically hard to guess secret string.
+   *   -bin. The storage bin. Multiple storage objects can be instantiated with
+   *   the same configuration, but for different bins.
+   */
+  public function __construct(array $configuration) {
+    parent::__construct($configuration);
+    $this->secret = $configuration['secret'];
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::save().
+   */
+  public function save($name, $data) {
+    $this->ensureDirectory();
+
+    // Write the file out to a temporary location. Prepend with a '.' to keep it
+    // hidden from listings and web servers.
+    $temporary_path = $this->directory . '/.' . str_replace('/', '#', $name);
+    if (!@file_put_contents($temporary_path, $data)) {
+      return FALSE;
+    }
+    chmod($temporary_path, 0400);
+
+    // 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
+    // mtime remains the same (unless the clock ticks to the next second during
+    // 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);
+    }
+
+    // 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
+    // update the directory mtime. However, this update will very likely not
+    // show up, because it has a coarse, one second granularity and typical
+    // moves takes significantly less than that. In the unlucky case the clock
+    // ticks during the move, we need to keep trying until the mtime we hashed
+    // on and the updated mtime match.
+    $previous_mtime = 0;
+    $i = 0;
+    while (($mtime = $this->getUncachedMTime($directory)) && ($mtime != $previous_mtime)) {
+      $previous_mtime = $mtime;
+      chmod($directory, 0300);
+      // Reset the file back in the temporary location if this is not the first
+      // iteration.
+      if ($i > 0) {
+        rename($full_path, $temporary_path);
+        // Make sure to not loop infinitely on a hopelessly slow filesystem.
+        if ($i > 10) {
+          unlink($temporary_path);
+          return FALSE;
+        }
+      }
+      $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);
+    file_save_htaccess($this->directory);
+  }
+
+  /**
+   * Removes everything in a directory, leaving it empty.
+   *
+   * @param $directory
+   *   The directory to be emptied out.
+   */
+  protected function cleanDirectory($directory) {
+    chmod($directory, 0700);
+    foreach (new DirectoryIterator($directory) as $fileinfo) {
+      if (!$fileinfo->isDot()) {
+        unlink($fileinfo->getPathName());
+      }
+    }
+  }
+
+  /**
+   * Returns the full path where the file is or should be stored.
+   *
+   * This function creates a file path that includes a unique containing
+   * directory for the file and a file name that is a hash of the virtual file
+   * name, a cryptographic secret, and the containing directory mtime. If the
+   * file is overridden by an insecure upload script, the directory mtime gets
+   * modified, invalidating the file, thus protecting against untrusted code
+   * getting executed.
+   *
+   * @param string $name
+   *   The virtual file name. Can be a relative path.
+   * @param string $directory
+   *   (optional) The directory containing the file. If not passed, this is
+   *   retrieved by calling getContainingDirectoryFullPath().
+   * @param int $directory_mtime
+   *   (optional) The mtime of $directory. Can be passed to avoid an extra
+   *   filesystem call when the mtime of the directory is already known.
+   * @return string
+   *    The full path where the file is or should be stored.
+   */
+  protected function getFullPath($name, &$directory = NULL, &$directory_mtime = NULL) {
+    if (!isset($directory)) {
+      $directory = $this->getContainingDirectoryFullPath($name);
+    }
+    if (!isset($directory_mtime)) {
+      $directory_mtime = file_exists($directory) ? filemtime($directory) : 0;
+    }
+    return $directory . '/' . hash_hmac('sha256', $name, $this->secret . $directory_mtime) . '.php';
+  }
+
+  /**
+   * Returns the full path of the containing directory where the file is or should be stored.
+   */
+  protected function getContainingDirectoryFullPath($name) {
+    return $this->directory . '/' . str_replace('/', '#', $name);
+  }
+
+  /**
+   * Clears PHP's stat cache and returns the directory's mtime.
+   */
+  protected function getUncachedMTime($directory) {
+    clearstatcache();
+    return filemtime($directory);
+  }
+}

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

+<?php
+
+/**
+ * @file
+ * Definition of Drupal\Component\PhpStorage\MTimeProtectedFileStorage.
+ */
+namespace Drupal\Component\PhpStorage;
+
+/**
+ * Stores PHP code in files with securely hashed names.
+ *
+ * The goal of this class is to ensure that if a PHP file is replaced with
+ * an untrusted one, it does not get loaded. Since mtime granularity is 1
+ * second, we cannot prevent an attack that happens within one second of the
+ * initial save(). However, it is very unlikely for an attacker exploiting an
+ * upload or file write vulnerability to also know when a legitimate file is
+ * being saved, discover its hash, undo its file permissions, and override the
+ * file with an upload all within a single second. Being able to accomplish
+ * that would indicate a site very likely vulnerable to many other attack
+ * vectors.
+ *
+ * Each file is stored in its own unique containing directory. The hash is
+ * based on the virtual file name, the containing directory's mtime, and a
+ * cryptographically hard to guess secret string. Thus, even if the hashed file
+ * name is discovered and replaced by an untrusted file (e.g., via a
+ * move_uploaded_file() invocation by a script that performs insufficient
+ * validation), the directory's mtime gets updated in the process, invalidating
+ * the hash and preventing the untrusted file from getting loaded. Also, the
+ * file mtime will be checked providing security against overwriting in-place,
+ * at the cost of an additional system call for every load() and exists().
+ *
+ * The containing directory is created with the same name as the virtual file
+ * name (slashes replaced with hashmarks) to assist with debugging, since the
+ * file itself is stored with a name that's meaningless to humans.
+ */
+class MTimeProtectedFileStorage extends MTimeProtectedFastFileStorage {
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load().
+   */
+  public function load($name) {
+    if (($filename = $this->checkFile($name)) !== FALSE) {
+      // Inline parent::load() to avoid an expensive getFullPath() call.
+      return (@include_once $filename) !== FALSE;
+    }
+    return FALSE;
+  }
+
+  /**
+   * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists().
+   */
+  public function exists($name) {
+    return $this->checkFile($name) !== FALSE;
+  }
+
+  /**
+   * Determines whether a protected file exists and sets the filename too.
+   *
+   * @param string $name
+   *   The virtual file name. Can be a relative path.
+   * return string
+   *   The full path where the file is if it is valid, FALSE otherwise.
+   */
+  protected function checkFile($name) {
+    $filename = $this->getFullPath($name, $directory, $directory_mtime);
+    return file_exists($filename) && filemtime($filename) <= $directory_mtime ? $filename : FALSE;
+  }
+}

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

+<?php
+
+/**
+ * @file
+ * Definition of Drupal\Component\PhpStorage\PhpStorageInterface.
+ */
+
+namespace Drupal\Component\PhpStorage;
+
+/**
+ * Stores and loads PHP code.
+ *
+ * Each interface function takes $name as a parameter. This is a virtual file
+ * name: for example, 'foo.php' or 'some/relative/path/to/foo.php'. The
+ * storage implementation may store these as files within the local file system,
+ * use a remote stream, combine multiple virtual files into an archive, store
+ * them in database records, or use some other storage technique.
+ */
+interface PhpStorageInterface {
+
+  /**
+   * Checks whether the PHP code exists in storage.
+   *
+   * @param string $name
+   *   The virtual file name. Can be a relative path.
+   *
+   * @return bool
+   *   TRUE if the virtual file exists, FALSE otherwise.
+   */
+  public function exists($name);
+
+  /**
+   * Loads PHP code from storage.
+   *
+   * Depending on storage implementation, exists() checks can be expensive, so
+   * this function may be called for a file that doesn't exist, and that should
+   * not result in errors. This function does not return anything, so it is
+   * up to the caller to determine if any code was loaded (for example, check
+   * class_exists() or function_exists() for what was expected in the code).
+   *
+   * @param string $name
+   *   The virtual file name. Can be a relative path.
+   */
+  public function load($name);
+
+  /**
+   * Saves PHP code to storage.
+   *
+   * @param string $name
+   *   The virtual file name. Can be a relative path.
+   * @param string $code
+   *    The PHP code to be saved.
+   *
+   * @return bool
+   *   TRUE if the save succeeded, FALSE if it failed.
+   */
+  public function save($name, $code);
+
+  /**
+   * Deletes PHP code from storage.
+   *
+   * @param string $name
+   *   The virtual file name. Can be a relative path.
+   *
+   * @return bool
+   *   TRUE if the delete succeeded, FALSE if it failed.
+   */
+  public function delete($name);
+}

core/modules/simpletest/lib/Drupal/simpletest/TestBase.php

@@ -761,7 +761,7 @@ protected function tearDown() {
     }
 
     // Delete temporary files directory.
-    file_unmanaged_delete_recursive($this->originalFileDirectory . '/simpletest/' . substr($this->databasePrefix, 10));
+    file_unmanaged_delete_recursive($this->originalFileDirectory . '/simpletest/' . substr($this->databasePrefix, 10), array($this, 'filePreDeleteCallback'));
 
     // Restore original database connection.
     Database::removeConnection('default');
@@ -939,4 +939,14 @@ public static function generatePermutations($parameters) {
     }
     return $all_permutations;
   }
+
+  /**
+   * Ensures test files are deletable within file_unmanaged_delete_recursive().
+   *
+   * Some tests chmod generated files to be read only. During tearDown() and
+   * other cleanup operations, these files need to get deleted too.
+   */
+  public static function filePreDeleteCallback($path) {
+    chmod($path, 0700);
+  }
 }

core/modules/simpletest/simpletest.module

@@ -507,7 +507,7 @@ function simpletest_clean_temporary_directories() {
     foreach ($files as $file) {
       $path = 'public://simpletest/' . $file;
       if (is_dir($path) && (is_numeric($file) || strpos($file, 'config_simpletest') !== FALSE)) {
-        file_unmanaged_delete_recursive($path);
+        file_unmanaged_delete_recursive($path, array('Drupal\simpletest\TestBase', 'filePreDeleteCallback'));
         $count++;
       }
     }

core/modules/system/lib/Drupal/system/Tests/PhpStorage/FileStorageTest.php

+<?php
+
+/**
+ * @file
+ * Definition of Drupal\system\Tests\PhpStorage\FileStorageTest.
+ */
+
+namespace Drupal\system\Tests\PhpStorage;
+
+/**
+ * Tests the simple file storage.