From af5201d492e07457a96fed6f839003e1c9ef3456 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 5 Feb 2014 13:36:08 -0800
Subject: [PATCH] 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
---
 CRM/Utils/Check/Security.php | 77 ++++++++++++++++++------------------
 CRM/Utils/File.php           | 24 +++++++++++
 2 files changed, 62 insertions(+), 39 deletions(-)

diff --git a/CRM/Utils/Check/Security.php b/CRM/Utils/Check/Security.php
index 7390063443..30c1e5ce41 100644
--- 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 bdea90483f..10c14de8e5 100644
--- 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
-- 
2.25.1