Create index.htm files in all tmp/ sub-folder as an additional safety…

… net (#10414) * Create index.htm files in tmp/ folder as safe net * UI test * silent fail * fix unit test * Minor Improvement to description * Fix release checklist * Fix release checklist * UI test * UI test logic * Actually make methods public to keep BC
matomo-org · Aug 23, 2016 · 58d7fb3 · 58d7fb3
1 parent c941d6a
commit 58d7fb3
Show file tree

Hide file tree

Showing 13 changed files with 100 additions and 100 deletions.
diff --git a/core/Filesystem.php b/core/Filesystem.php
@@ -94,6 +94,8 @@ public static function mkdir($path)
                 // enough! we're not going to make the directory world-writeable
             }
         }
+
+        self::createIndexFilesToPreventDirectoryListing($path);
     }
 
     /**
@@ -443,8 +445,7 @@ public static function remove($file, $silenceErrors = false)
      */
     private static function getChmodForPath($path)
     {
-        $pathIsTmp = StaticContainer::get('path.tmp');
-        if (strpos($path, $pathIsTmp) === 0) {
+        if (self::isPathWithinTmpFolder($path)) {
             // tmp/* folder
             return 0750;
         }
@@ -504,4 +505,34 @@ private static function tryToCopyFileAndVerifyItWasCopied($source, $dest)
 
         return true;
     }
+
+    /**
+     * @param $path
+     * @return bool
+     */
+    private static function isPathWithinTmpFolder($path)
+    {
+        $pathIsTmp = StaticContainer::get('path.tmp');
+        $isPathWithinTmpFolder = strpos($path, $pathIsTmp) === 0;
+        return $isPathWithinTmpFolder;
+    }
+
+    /**
+     * in tmp/ (sub-)folder(s) we create empty index.htm|php files
+     *
+     * @param $path
+     */
+    private static function createIndexFilesToPreventDirectoryListing($path)
+    {
+        if (!self::isPathWithinTmpFolder($path)) {
+            return;
+        }
+        $filesToCreate = array(
+            $path . '/index.htm',
+            $path . '/index.php'
+        );
+        foreach ($filesToCreate as $file) {
+            @file_put_contents($file, 'Nothing to see here.');
+        }
+    }
 }
diff --git a/core/Updates/2.15.0-b4.php b/core/Updates/2.15.0-b4.php
diff --git a/core/Updates/2.16.1-b3.php b/core/Updates/2.16.1-b3.php
diff --git a/core/Updates/2.3.0-rc2.php → core/Updates/2.16.3-b1.php b/core/Updates/2.3.0-rc2.php → core/Updates/2.16.3-b1.php
@@ -6,20 +6,17 @@
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
  *
  */
+
 namespace Piwik\Updates;
 
 use Piwik\Plugins\Installation\ServerFilesGenerator;
-use Piwik\Updates;
 use Piwik\Updater;
+use Piwik\Updates as PiwikUpdates;
 
-/**
- */
-class Updates_2_3_0_rc2 extends Updates
+class Updates_2_16_3_b1 extends PiwikUpdates
 {
     public function doUpdate(Updater $updater)
     {
-        ServerFilesGenerator::deleteHtAccessFiles();
-
-        ServerFilesGenerator::createHtAccessFiles();
+        ServerFilesGenerator::createFilesForSecurity();
     }
 }
diff --git a/core/Updates/2.4.0-b2.php b/core/Updates/2.4.0-b2.php
diff --git a/core/Version.php b/core/Version.php
@@ -20,7 +20,7 @@ final class Version
      * The current Piwik version.
      * @var string
      */
-    const VERSION = '2.16.2';
+    const VERSION = '2.16.3-b1';
 
     public function isStableVersion($version)
     {

diff --git a/plugins/Installation/Controller.php b/plugins/Installation/Controller.php
@@ -300,6 +300,8 @@ public function firstWebsiteSetup()
     {
         $this->checkPiwikIsNotInstalled();
 
+        ServerFilesGenerator::createFilesForSecurity();
+
         $siteIdsCount = Access::doAsSuperUser(function () {
             return count(APISitesManager::getInstance()->getAllSitesId());
         });
@@ -720,4 +722,5 @@ protected function updateComponents()
             return $result;
         });
     }
+
 }
diff --git a/plugins/Installation/ServerFilesGenerator.php b/plugins/Installation/ServerFilesGenerator.php
@@ -13,6 +13,17 @@
 
 class ServerFilesGenerator
 {
+    public static function createFilesForSecurity()
+    {
+        self::deleteHtAccessFiles();
+        self::createHtAccessFiles();
+
+        self::deleteWebConfigFiles();
+        self::createWebConfigFiles();
+
+        self::createWebRootFiles();
+    }
+
     /**
      * Generate Apache .htaccess files to restrict access
      * .htaccess files are created on all webservers even Nginx, as sometimes Nginx knows how to handle .htaccess files
@@ -64,11 +75,6 @@ public static function createHtAccessFiles()
         }
     }
 
-    public static function createHtAccessDenyAll($path)
-    {
-        self::createHtAccess($path, $overwrite = false, self::getDenyAllHtaccessContent());
-    }
-
     /**
      * Create .htaccess file in specified directory
      *
@@ -83,6 +89,8 @@ public static function createHtAccessDenyAll($path)
     protected static function createHtAccess($path, $overwrite = true, $content)
     {
         $file = $path . '/.htaccess';
+
+        $content = "# This file is auto generated by Piwik, do not edit directly\n# Please report any issue or improvement directly to the Piwik team.\n\n" . $content;
         if ($overwrite || !file_exists($file)) {
             @file_put_contents($file, $content);
         }
@@ -93,7 +101,7 @@ protected static function createHtAccess($path, $overwrite = true, $content)
      *
      * Note: for IIS 7 and above
      */
-    public static function createWebConfigFiles()
+    protected static function createWebConfigFiles()
     {
         if (!SettingsServer::isIIS()) {
             return;
@@ -183,7 +191,10 @@ public static function createWebRootFiles()
             '/favicon.ico',
         );
         foreach ($filesToCreate as $file) {
-            @file_put_contents(PIWIK_DOCUMENT_ROOT . $file, '');
+            $path = PIWIK_DOCUMENT_ROOT . $file;
+            if(!file_exists($path)) {
+                @file_put_contents($path, '');
+            }
         }
     }
 

diff --git a/tests/PHPUnit/Integration/ReleaseCheckListTest.php b/tests/PHPUnit/Integration/ReleaseCheckListTest.php
@@ -257,6 +257,7 @@ public function test_phpFilesStartWithRightCharacter()
     {
         $files = Filesystem::globr(PIWIK_INCLUDE_PATH, '*.php');
 
+        $tested = 0;
         foreach($files as $file) {
             // skip files in these folders
             if (strpos($file, '/libs/') !== false) {
@@ -279,7 +280,10 @@ public function test_phpFilesStartWithRightCharacter()
 
             $start = fgets($handle, strlen($expectedStart) + 1 );
             $this->assertEquals($start, $expectedStart, "File $file does not start with $expectedStart");
+            $tested++;
         }
+
+        $this->assertGreaterThan(2000, $tested, 'should have tested at least thousand of  php files');
     }
 
     public function test_jsfilesDoNotContainFakeSpaces()
@@ -494,7 +498,8 @@ protected function isSkipPhpFileStartWithPhpBlock($file, $isIniFile)
             || strpos($file, "tests/resources/Updater/") !== false
             || strpos($file, "Twig/Tests/") !== false
             || strpos($file, "processed/") !== false
-            || strpos($file, "/vendor/") !== false;
+            || strpos($file, "/vendor/") !== false
+            || (strpos($file, "tmp/") !== false && strpos($file, 'index.php') !== false);
         $isLib = strpos($file, "lib/xhprof") !== false || strpos($file, "phpunit/phpunit") !== false;
 
         return ($isIniFile && $isIniFileInTests) || $isTestResultFile || $isLib;

diff --git a/tests/PHPUnit/Unit/FilesystemTest.php b/tests/PHPUnit/Unit/FilesystemTest.php
@@ -93,24 +93,35 @@ public function test_directoryDiff_shouldReturnAllTargetFiles_IfSourceIsEmpty()
             '/DataTable/Bridges.php',
             '/DataTable/DataTableInterface.php',
             '/DataTable/Filter',
+            '/DataTable/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
             '/DataTable/Manager.php',
             '/DataTable/Map.php',
             '/DataTable/Renderer',
             '/DataTable/Renderer.php',
             '/DataTable/Row',
             '/DataTable/Row.php',
             '/DataTable/Simple.php',
+            '/DataTable/Filter/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Filter/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
             '/DataTable/Renderer/Console.php',
             '/DataTable/Renderer/Csv.php',
             '/DataTable/Renderer/Html.php',
+            '/DataTable/Renderer/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Renderer/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
             '/DataTable/Renderer/Json.php',
             '/DataTable/Renderer/Php.php',
             '/DataTable/Renderer/Rss.php',
             '/DataTable/Renderer/Tsv.php',
             '/DataTable/Renderer/Xml',
             '/DataTable/Renderer/Xml.php',
+            '/DataTable/Renderer/Xml/index.htm',  // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Renderer/Xml/index.php',  // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
             '/DataTable/Renderer/Xml/Other.php',
-            '/DataTable/Row/DataTableSummaryRow.php'
+            '/DataTable/Row/DataTableSummaryRow.php',
+            '/DataTable/Row/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Row/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+
         ), $result);
     }
 
@@ -121,12 +132,18 @@ public function test_directoryDiff_shouldReturnFilesPresentInTargetButNotSource_
         $this->assertEquals(array(
             '/DataTable/Filter',
             '/DataTable/Row',
+            '/DataTable/Filter/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Filter/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
             '/DataTable/Renderer/Json.php',
             '/DataTable/Renderer/Php.php',
             '/DataTable/Renderer/Rss.php',
             '/DataTable/Renderer/Xml',
+            '/DataTable/Renderer/Xml/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Renderer/Xml/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
             '/DataTable/Renderer/Xml/Other.php',
             '/DataTable/Row/DataTableSummaryRow.php',
+            '/DataTable/Row/index.htm', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
+            '/DataTable/Row/index.php', // this was is created as side effect of "Target files" being within the tmp/ folder, @see createIndexFilesToPreventDirectoryListing
         ), $result);
     }
 
@@ -137,7 +154,7 @@ public function test_unlinkTargetFilesNotPresentInSource_shouldUnlinkFilesPresen
 
         // make sure there is a difference between those folders
         $result = Filesystem::directoryDiff($source, $target);
-        $this->assertCount(8, $result);
+        $this->assertCount(14, $result);
 
         Filesystem::unlinkTargetFilesNotPresentInSource($source, $target);
 

diff --git a/tests/UI/expected-ui-screenshots b/tests/UI/expected-ui-screenshots
diff --git a/tests/UI/specs/Installation_spec.js b/tests/UI/specs/Installation_spec.js
@@ -51,8 +51,22 @@ describe("Installation", function () {
         }, done);
     });
 
+    var pageUrl;
+    it("should have already created a tmp/sessions/index.htm file to prevent directory listing", function (done) {
+        expect.screenshot('nothing_to_see_here').to.be.capture(function (page) {
+            pageUrl = page.getCurrentUrl();
+
+            // page.load will load by default the proxy ie. http://localhost/piwik/tests/PHPUnit/proxy/
+            // but we need here to check in: http://localhost/piwik/tmp/sessions/
+            page.load("../../../tmp/sessions/index.htm");
+
+        }, done);
+    });
+
     it("should display the database setup page when next is clicked on the system check page", function (done) {
         expect.screenshot("db_setup").to.be.capture(function (page) {
+            page.load(pageUrl);
+
             page.click('.next-step .btn');
         }, done);
     });

diff --git a/tests/index.html b/tests/index.html
@@ -6,8 +6,7 @@
 
 <ul>
     <li><a href="https://github.com/piwik/piwik/blob/master/tests/README.md">Setup PHPUnit tests</a></li>
-    <li><a href="javascript/">Run piwik.js Javascript unit & integration tests</a>. <br/><i>Note: the Javascript tests
-            are not executed in Jenkins so must be run manually on major browsers after any change to piwik.js</i></li>
+    <li><a href="javascript/">Run piwik.js Javascript unit & integration tests</a>. <br/></li>
 </ul>
 
 <p>If you are new to the wonderful world of testing, <a href='https://github.com/piwik/piwik/blob/master/tests/README.md'>see the README</a> for an introduction.</p>