Merge pull request #48 from silverstripe-security/pulls/4.0/fix-install-redacting

[SS-2017-010] Prevent install.php from disclosing system passwords

src/Dev/Install/InstallConfig.php

@@ -26,18 +26,23 @@ class InstallConfig
      *
      * @param array $request Request object
      * @param array $databaseClasses Supported database config
+     * @param bool $realPassword Set to true to get the real password. If false, any non-posted
+     * password will be redacted as '********'. Note: Posted passwords are considered disclosed and
+     * never redacted.
      * @return array
      */
-    public function getDatabaseConfig($request, $databaseClasses)
+    public function getDatabaseConfig($request, $databaseClasses, $realPassword = true)
     {
         // Get config from request
         if (isset($request['db']['type'])) {
             $type = $request['db']['type'];
             if (isset($request['db'][$type])) {
-                return array_merge(
+                $config = $request['db'][$type];
-                    [ 'type' => $type ],
+                // The posted placeholder must be substituted with the real password
-                    $request['db'][$type]
+                if (isset($config['password']) && $config['password'] === Installer::PASSWORD_PLACEHOLDER) {
-                );
+                    $config['password'] = Environment::getEnv('SS_DATABASE_PASSWORD') ?: '';
+                }
+                return array_merge([ 'type' => $type ], $config);
             }
         }
 
@@ -46,8 +51,16 @@ class InstallConfig
             'type' => $this->getDatabaseClass($databaseClasses),
             'server' => Environment::getEnv('SS_DATABASE_SERVER') ?: 'localhost',
             'username' => Environment::getEnv('SS_DATABASE_USERNAME') ?: 'root',
-            'password' => Environment::getEnv('SS_DATABASE_PASSWORD') ?: '',
+            'password' => $realPassword
+                ? (Environment::getEnv('SS_DATABASE_PASSWORD') ?: '')
+                : Installer::PASSWORD_PLACEHOLDER, // Avoid password disclosure
             'database' => Environment::getEnv('SS_DATABASE_NAME') ?: 'SS_mysite',
+            'path' => Environment::getEnv('SS_DATABASE_PATH')
+                ?: Environment::getEnv('SS_SQLITE_DATABASE_PATH') // sqlite compat
+                ?: null,
+            'key' => Environment::getEnv('SS_DATABASE_KEY')
+                ?: Environment::getEnv('SS_SQLITE_DATABASE_KEY') // sqlite compat
+                ?: null,
         ];
     }
 
@@ -55,17 +68,26 @@ class InstallConfig
      * Get admin config from the environment
      *
      * @param array $request
+     * @param bool $realPassword Set to true to get the real password. If false, any non-posted
+     * password will be redacted as '********'. Note: Posted passwords are considered disclosed and
+     * never redacted.
      * @return array
      */
-    public function getAdminConfig($request)
+    public function getAdminConfig($request, $realPassword = true)
     {
         if (isset($request['admin'])) {
+            $config = $request['admin'];
+            if (isset($config['password']) && $config['password'] === Installer::PASSWORD_PLACEHOLDER) {
+                $config['password'] = Environment::getEnv('SS_DEFAULT_ADMIN_PASSWORD') ?: '';
+            }
             return $request['admin'];
         }
 
         return [
             'username' => Environment::getEnv('SS_DEFAULT_ADMIN_USERNAME') ?: 'admin',
-            'password' => Environment::getEnv('SS_DEFAULT_ADMIN_PASSWORD') ?: '',
+            'password' => $realPassword
+                ? (Environment::getEnv('SS_DEFAULT_ADMIN_PASSWORD') ?: '')
+                : Installer::PASSWORD_PLACEHOLDER, // Avoid password disclosure
         ];
     }
 

src/Dev/Install/Installer.php

@@ -20,6 +20,11 @@ use SilverStripe\Security\Security;
  */
 class Installer extends InstallRequirements
 {
+    /**
+     * value='' attribute placeholder for password fields
+     */
+    const PASSWORD_PLACEHOLDER = '********';
+
     protected function installHeader()
     {
         ?>

src/Dev/Install/config-form.html

@@ -123,14 +123,15 @@
 												echo $checked ? '<div class="dbfields">' : '<div class="dbfields" style="display: none;">';
 												if(isset($details['fields'])) foreach($details['fields'] as $fieldName => $fieldSpec) {
 													$fieldTitle = $fieldSpec['title'];
-													$fieldType = ($fieldName == 'password') ? 'password' : 'text';
+													$fieldType = ($fieldName === 'password') ? 'password' : 'text';
-													// values
+                                                    // Get display value
-													$defaultValue = (isset($fieldSpec['default'])) ? $fieldSpec['default'] : null;
+                                                    if (isset($databaseConfig[$fieldName]) && $databaseConfig['type'] == $class) {
-													if($usingEnv && isset($fieldSpec['envVar']) && $envVar = getenv($fieldSpec['envVar'])) {
+                                                        $value = $databaseConfig[$fieldName];
-														$value = $envVar;
+                                                    } elseif (isset($fieldSpec['default'])) {
-													} else {
+                                                        $value = $fieldSpec['default'];
-														$value = (isset($databaseConfig[$fieldName]) && $databaseConfig['type'] == $class) ? $databaseConfig[$fieldName] : $defaultValue;
+                                                    } else {
-													}
+                                                        $value = null;
+                                                    }
 
 													// attributes
 													$attrs = array(

src/Dev/Install/install5.php

@@ -56,8 +56,8 @@ foreach ($databaseClasses as $class => $details) {
 
 // Build config from config / environment / request
 $config = new InstallConfig();
-$databaseConfig = $config->getDatabaseConfig($_REQUEST, $databaseClasses);
+$databaseConfig = $config->getDatabaseConfig($_REQUEST, $databaseClasses, true);
-$adminConfig = $config->getAdminConfig($_REQUEST);
+$adminConfig = $config->getAdminConfig($_REQUEST, true);
 $alreadyInstalled = $config->alreadyInstalled();
 $silverstripe_version = $config->getFrameworkVersion();
 $sendStats = $config->canSendStats($_REQUEST);
@@ -107,22 +107,6 @@ if ($installFromCli && ($req->hasErrors() || $dbReq->hasErrors())) {
 // Path to client resources (copied through silverstripe/vendor-plugin)
 $clientPath = 'resources/silverstripe/framework/src/Dev/Install/client';
 
-
-// config-form.html vars (placeholder to prevent deletion)
-[
-    $theme,
-    $clientPath,
-    $adminConfig,
-    $usingEnv,
-    $silverstripe_version,
-    $locale,
-    $locales,
-    $webserverConfigFile,
-    $hasErrorOtherThanDatabase,
-    $hasOnlyWarnings, // If warnings but not errors
-    $phpIniLocation
-];
-
 // If already installed, ensure the user clicked "reinstall"
 $expectedArg = $alreadyInstalled ? 'reinstall' : 'go';
 if ((isset($_REQUEST[$expectedArg]) || $installFromCli)
@@ -142,7 +126,27 @@ if ((isset($_REQUEST[$expectedArg]) || $installFromCli)
         'admin' => $adminConfig,
         'stats' => $sendStats,
     ]);
-    // Show the config form
+    return;
-} else {
-    include(__DIR__ . '/config-form.html');
 }
+
+// Sanitise config prior to rendering config-form.html
+$databaseConfig = $config->getDatabaseConfig($_REQUEST, $databaseClasses, false);
+$adminConfig = $config->getAdminConfig($_REQUEST, false);
+
+// config-form.html vars (placeholder to prevent deletion)
+[
+    $theme,
+    $clientPath,
+    $adminConfig,
+    $databaseConfig,
+    $usingEnv,
+    $silverstripe_version,
+    $locale,
+    $locales,
+    $webserverConfigFile,
+    $hasErrorOtherThanDatabase,
+    $hasOnlyWarnings, // If warnings but not errors
+    $phpIniLocation,
+];
+
+include(__DIR__ . '/config-form.html');