CRM-14092 - Restrict browsing of imageUploadDir via imageUploadURL

Previously, it attempted to restrict browsing of uploadDir and
configAndLogDir.  However, this is extraneoous because we have other checks
to ensure that those directories are inaccessible.  However, imageUploadDir
is different because we want to expose its file -- we just don't want to
expose a listing of them.

This commit also breaks out checkDirectoriesAreNotBrowseable() into
three functions.

----------------------------------------
* CRM-14092:
  http://issues.civicrm.org/jira/browse/CRM-14092

diff --git a/CRM/Utils/Check/Security.php b/CRM/Utils/Check/Security.php
index 739006344333f128386ba5eb03b1afa8274091c0..30c1e5ce414a286e048ad5c5b06ae6ed4eadf8c2 100644 (file)
--- a/CRM/Utils/Check/Security.php
+++ b/CRM/Utils/Check/Security.php
@@ -224,53 +224,52 @@ class CRM_Utils_Check_Security {
    */
   public function checkDirectoriesAreNotBrowseable() {
     $messages = array();
-
     $config = CRM_Core_Config::singleton();
-    $log = CRM_Core_Error::createDebugLogger();
-    $log_name = $log->_filename;
-    $filePathMarker = $this->getFilePathMarker();
-
-    $paths = array(
-      $config->uploadDir,
-      dirname($log_name),
+    $publicDirs = array(
+      $config->imageUploadDir => $config->imageUploadURL,
     );
-    if ($upload_url = explode($filePathMarker, $config->imageUploadURL)) {
-      if ($files = glob($config->uploadDir . '/*')) {
-        foreach ($paths as $path) {
-          // Since we're here ...
-          $dir = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path));
-          foreach ($dir as $name => $object) {
-            if (is_dir($name) && $name != '..') {
-              $nobrowse = realpath($name) . '/index.html';
-              if (!file_exists($nobrowse)) {
-                @file_put_contents($nobrowse, '');
-              }
-            }
-          }
-          // OK, now let's see if it's browseable.
-          if ($dir_path = explode($filePathMarker, $path)) {
-            $url = implode($filePathMarker, array($upload_url[0], $dir_path[1]));
-            if ($files = glob($path . '/*')) {
-              if ($listing = @file_get_contents($url)) {
-                foreach ($files as $file) {
-                  if (stristr($listing, $file)) {
-                    $msg = 'Directory <a href="%1">%2</a> may be browseable via the web.'
-                      . '<br />' .
-                      '<a href="%3">Read more about this warning</a>';
-                    $docs_url = $this->createDocUrl('checkDirectoriesAreNotBrowseable');
-                    $messages[] = ts($msg, array(1 => $log_url, 2 => $path, 3 => $docs_url));
-                  }
-                }
-              }
-            }
-          }
-        }
+
+    // Setup index.html files to prevent browsing
+    foreach ($publicDirs as $publicDir => $publicUrl) {
+      CRM_Utils_File::restrictBrowsing($publicDir);
+    }
+
+    // Test that $publicDir is not browsable
+    foreach ($publicDirs as $publicDir => $publicUrl) {
+      if ($this->isBrowsable($publicDir, $publicUrl)) {
+        $msg = 'Directory <a href="%1">%2</a> should not be browseable via the web.'
+          . '<br />' .
+          '<a href="%3">Read more about this warning</a>';
+        $docs_url = $this->createDocUrl('checkDirectoriesAreNotBrowseable');
+        $messages[] = ts($msg, array(1 => $publicDir, 2 => $publicDir, 3 => $docs_url));
       }
     }
 
     return $messages;
   }
 
+  /**
+   * Determine whether $url is a public, browsable listing for $dir
+   *
+   * @param string $dir local dir path
+   * @param string $url public URL
+   * @return bool
+   */
+  public function isBrowsable($dir, $url) {
+    $result = FALSE;
+    $file = 'delete-this-' . CRM_Utils_String::createRandom(10, CRM_Utils_String::ALPHANUMERIC);
+
+    // this could be a new system with uploads yet -- so we'll make a file
+    file_put_contents("$dir/$file", "delete me");
+    $content = file_get_contents("$url");
+    if (stristr($content, $file)) {
+      $result = TRUE;
+    }
+    unlink("$dir/$file");
+
+    return $result;
+  }
+
   public function createDocUrl($topic) {
     return CRM_Utils_System::getWikiBaseURL() . $topic;
   }

diff --git a/CRM/Utils/File.php b/CRM/Utils/File.php
index bdea90483fd7767d1120b453a0bb7a07fc8de139..10c14de8e591fd57c3b73371f03b532c36f43e56 100644 (file)
--- a/CRM/Utils/File.php
+++ b/CRM/Utils/File.php
@@ -399,6 +399,30 @@ HTACCESS;
     }
   }
 
+  /**
+   * Restrict remote users from browsing the given directory.
+   *
+   * @param $publicDir
+   */
+  static function restrictBrowsing($publicDir) {
+    // base dir
+    $nobrowse = realpath($publicDir) . '/index.html';
+    if (!file_exists($nobrowse)) {
+      @file_put_contents($nobrowse, '');
+    }
+
+    // child dirs
+    $dir = new RecursiveDirectoryIterator($publicDir);
+    foreach ($dir as $name => $object) {
+      if (is_dir($name) && $name != '..') {
+        $nobrowse = realpath($name) . '/index.html';
+        if (!file_exists($nobrowse)) {
+          @file_put_contents($nobrowse, '');
+        }
+      }
+    }
+  }
+
   /**
    * Create the base file path from which all our internal directories are
    * offset. This is derived from the template compile directory set