From 3b1a1ae7e309b8ccae1fc1aa148d00e604530ccc Mon Sep 17 00:00:00 2001
From: James Rhodes <jrhodes@redpointsoftware.com.au>
Date: Sat, 6 Apr 2013 00:27:33 -0700
Subject: [PATCH] [SECURITY] Prevented PhabricatorSetupIssueView from exposing
 sensitive config options.

Summary:
Currently PhabricatorSetupIssueView will show the current values of
configuration options regardless of whether or not they are defined
as hidden options.  This means that if the MySQL server stops, Phabricator
will present the MySQL connection credentials to anyone who can access
the Phabricator page.

Test Plan:
Stop the MySQL server for a Phabricator instance.  It should display 'hidden'
instead of the MySQL password.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5596
---
 .../response/PhabricatorConfigResponse.php    |  2 +-
 .../config/view/PhabricatorSetupIssueView.php | 28 +++++++++++++++----
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/applications/config/response/PhabricatorConfigResponse.php b/src/applications/config/response/PhabricatorConfigResponse.php
index 923313bb2a..5ae68f2c6d 100644
--- a/src/applications/config/response/PhabricatorConfigResponse.php
+++ b/src/applications/config/response/PhabricatorConfigResponse.php
@@ -50,7 +50,7 @@ final class PhabricatorConfigResponse extends AphrontHTMLResponse {
       $resources[] = phutil_tag(
         'style',
         array('type' => 'text/css'),
-        Filesystem::readFile($webroot.'/rsrc/css/'.$path));
+        phutil_safe_html(Filesystem::readFile($webroot.'/rsrc/css/'.$path)));
     }
     return phutil_implode_html("\n", $resources);
   }
diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php
index c5a0932e3e..b57d0c0b19 100644
--- a/src/applications/config/view/PhabricatorSetupIssueView.php
+++ b/src/applications/config/view/PhabricatorSetupIssueView.php
@@ -132,13 +132,25 @@ final class PhabricatorSetupIssueView extends AphrontView {
         "The current Phabricator configuration has these %d value(s):",
         count($configs)));
 
+    $options = PhabricatorApplicationConfigOptions::loadAllOptions();
+    $hidden = array();
+    foreach ($options as $key => $option) {
+      if ($option->getHidden()) {
+        $hidden[$key] = true;
+      }
+    }
+
+    $table = null;
     $dict = array();
     foreach ($configs as $key) {
-      $dict[$key] = PhabricatorEnv::getUnrepairedEnvConfig($key);
+      if (isset($hidden[$key])) {
+        $dict[$key] = null;
+      } else {
+        $dict[$key] = PhabricatorEnv::getUnrepairedEnvConfig($key);
+      }
     }
-    $table = $this->renderValueTable($dict);
 
-    $options = PhabricatorApplicationConfigOptions::loadAllOptions();
+    $table = $this->renderValueTable($dict, $hidden);
 
     if ($this->getIssue()->getIsFatal()) {
       $update_info = phutil_tag(
@@ -299,12 +311,18 @@ final class PhabricatorSetupIssueView extends AphrontView {
       ));
   }
 
-  private function renderValueTable(array $dict) {
+  private function renderValueTable(array $dict, array $hidden = array()) {
     $rows = array();
     foreach ($dict as $key => $value) {
+      if (isset($hidden[$key])) {
+        $value = phutil_tag('em', array(), 'hidden');
+      } else {
+        $value = $this->renderValueForDisplay($value);
+      }
+
       $cols = array(
         phutil_tag('th', array(), $key),
-        phutil_tag('td', array(), $this->renderValueForDisplay($value)),
+        phutil_tag('td', array(), $value),
       );
       $rows[] = phutil_tag('tr', array(), $cols);
     }