From ffc8433468debe4283faed34ef2ffc7893c9deca Mon Sep 17 00:00:00 2001 From: Bob Vincent Date: Tue, 24 Jan 2012 08:11:55 -0500 Subject: [PATCH] Issue #1008402 by tekante, pillarsdotnet, kathyh, ufku: Allow the use of symlinks within the files directory. --- core/lib/Drupal/Core/StreamWrapper/LocalStream.php | 30 +++++++++---- core/modules/simpletest/tests/file.test | 47 +++++++++++++++++++- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/core/lib/Drupal/Core/StreamWrapper/LocalStream.php b/core/lib/Drupal/Core/StreamWrapper/LocalStream.php index 9f2b95ded36d83455237ca256c988c56956ff8de..592d90f09d2c3bfe1993dc14e40405f70a21fc00 100644 --- a/core/lib/Drupal/Core/StreamWrapper/LocalStream.php +++ b/core/lib/Drupal/Core/StreamWrapper/LocalStream.php @@ -161,17 +161,29 @@ abstract class LocalStream implements StreamWrapperInterface { if (!isset($uri)) { $uri = $this->uri; } - $path = $this->getDirectoryPath() . '/' . $this->getTarget($uri); - $realpath = realpath($path); - if (!$realpath) { - // This file does not yet exist. - $realpath = realpath(dirname($path)) . '/' . drupal_basename($path); - } - $directory = realpath($this->getDirectoryPath()); - if (!$realpath || !$directory || strpos($realpath, $directory) !== 0) { + // Get the target path relative to the files repository. + $target = DIRECTORY_SEPARATOR . $this->getTarget($uri); + // Get the files repository directory. + $repository = realpath($this->getDirectoryPath()); + // Get the target directory. + $target_dir = realpath(dirname($repository . $target)) . DIRECTORY_SEPARATOR; + // Get the target name, without any directory components. + $target_name = drupal_basename($repository . $target); + // A directory component can point outside its parent directory if the path + // separator ('/' or '\') is followed either by '..' to reference the parent + // directory. + $pattern = '@(/|\\\\)\.\.@'; + // Check whether the target can possibly point outside its parent. + $traversal = preg_match($pattern, $target); + // Check whether the target dir exists within the files repository. + $subdirectory = strpos($target_dir, $repository . DIRECTORY_SEPARATOR) === 0; + if ($traversal && !$subdirectory) { + // If the target path contains directory-traversal parts such as '/..' and + // does not resolve to a subdirectory of the repository, then return FALSE + // to avoid a possible exploit. return FALSE; } - return $realpath; + return $target_dir . $target_name; } /** diff --git a/core/modules/simpletest/tests/file.test b/core/modules/simpletest/tests/file.test index 5b6a6f456aecbcc3d6e1c954db58269ca73858ec..5efdf3fe0fc64a640b447fdbdb6beffacf614e50 100644 --- a/core/modules/simpletest/tests/file.test +++ b/core/modules/simpletest/tests/file.test @@ -1034,6 +1034,42 @@ class FileDirectoryTest extends FileTestCase { $setting = variable_get('file_temporary_path', ''); $this->assertEqual($setting, $tmp_directory, t("The 'file_temporary_path' variable has the same value that file_directory_temp() returned.")); } + + /** + * Tests that symlinks are supported within the files directory. + */ + function testFileDirectorySymlinks() { + // Temporarily unset the file_temporary_path variable. + $file_temporary_path = variable_get('file_temporary_path', NULL); + variable_del('file_temporary_path'); + // Now the return value of file_directory_temp() should be outside + // the public files directory. + $temp = file_directory_temp(); + // Restore the file_temporary_path variable. + variable_set('file_temporary_path', $file_temporary_path); + $public = drupal_realpath('public://'); + $dirname = $this->randomname(20); + $filename = $this->randomname(20); + // Create a randomly-named directory in the temp folder. + $temp_dir = drupal_realpath($temp) . DIRECTORY_SEPARATOR . $dirname; + drupal_mkdir($temp_dir); + // Create a symlink in the public files folder pointing to the + // newly-created directory. + $symlink = $public . DIRECTORY_SEPARATOR . $dirname; + symlink($temp_dir, $symlink); + // Copy a test file to the symlinked directory. + $source = current($this->drupalGetTestFiles('text'))->uri; + $destination = "public://$dirname/$filename"; + file_unmanaged_copy($source, $destination, FILE_EXISTS_ERROR); + // Test that the real path of the copied file lies in the temp folder. + $realpath = drupal_realpath($destination); + $compare = $temp_dir . DIRECTORY_SEPARATOR . $filename; + $this->assertEqual($realpath, $compare, "drupal_realpath('$destination') returned '$realpath'; expected '$compare'"); + // Clean up the mess. + file_unmanaged_delete($destination); + drupal_unlink($symlink); + drupal_rmdir($temp_dir); + } } /** @@ -2421,8 +2457,11 @@ class FileDownloadTest extends FileTestCase { '%2523%2525%2526%252B%252F%253F' . '%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E'; - $this->checkUrl('public', '', $basename, $base_url . '/' . file_stream_wrapper_get_instance_by_scheme('public')->getDirectoryPath() . '/' . $basename_encoded); - $this->checkUrl('private', '', $basename, $base_url . '/system/files/' . $basename_encoded); + $this->checkUrl('public', '', $basename, $base_url . '/' . file_stream_wrapper_get_instance_by_scheme('public')->getDirectoryPath() . '/' . $basename_encoded, 0); + // The following test fails if clean urls are not enabled. + if (variable_get('clean_url', 0)) { + $this->checkUrl('private', '', $basename, $base_url . '/system/files/' . $basename_encoded); + } $this->checkUrl('private', '', $basename, $base_url . '/?q=system/files/' . $basename_encoded, '0'); } @@ -2445,6 +2484,8 @@ class FileDownloadTest extends FileTestCase { * The value of the clean_url setting */ private function checkUrl($scheme, $directory, $filename, $expected_url, $clean_url = '1') { + // Save the original clean_url value for later restoring. + $old_clean_url = variable_get('clean_url', 0); variable_set('clean_url', $clean_url); // Convert $filename to a valid filename, i.e. strip characters not @@ -2470,6 +2511,8 @@ class FileDownloadTest extends FileTestCase { } file_delete($file); + // Restore the original clean_url value. + variable_set('clean_url', $old_clean_url); } } -- 1.7.5.4