From e7702acdc65986b8db2616c47f74d77a378ac01c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 22 Apr 2015 09:28:35 -0700
Subject: [PATCH] Don't escape quotation marks when printing the monospaced CSS
 rule

Summary:
Fixes T7888. This is currently safe, but double quotes are incorrectly escaped.

To keep them unescaped, we have to punch through PhutilSafeHTML a bit. Since the allowable characters are strictly filtered this is still safe in practice, just not as theoretically-safe.

Test Plan: Set font to `32px "impact"` (with quotes), saw impact font.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7888

Differential Revision: https://secure.phabricator.com/D12506
---
 .../PhabricatorDisplayPreferencesSettingsPanel.php    |  5 ++---
 .../settings/storage/PhabricatorUserPreferences.php   |  5 +++++
 src/view/page/PhabricatorStandardPageView.php         | 11 +++++++++--
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/applications/settings/panel/PhabricatorDisplayPreferencesSettingsPanel.php b/src/applications/settings/panel/PhabricatorDisplayPreferencesSettingsPanel.php
index b58d22a748..9d1211aee5 100644
--- a/src/applications/settings/panel/PhabricatorDisplayPreferencesSettingsPanel.php
+++ b/src/applications/settings/panel/PhabricatorDisplayPreferencesSettingsPanel.php
@@ -30,9 +30,8 @@ final class PhabricatorDisplayPreferencesSettingsPanel
     $e_editor = null;
     if ($request->isFormPost()) {
       $monospaced = $request->getStr($pref_monospaced);
-
-      // Prevent the user from doing stupid things.
-      $monospaced = preg_replace('/[^a-z0-9 ,".]+/i', '', $monospaced);
+      $monospaced = PhabricatorUserPreferences::filterMonospacedCSSRule(
+        $monospaced);
 
       $preferences->setPreference($pref_titles, $request->getStr($pref_titles));
       $preferences->setPreference($pref_editor, $request->getStr($pref_editor));
diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php
index 2e1576653b..6e05e5e44c 100644
--- a/src/applications/settings/storage/PhabricatorUserPreferences.php
+++ b/src/applications/settings/storage/PhabricatorUserPreferences.php
@@ -101,4 +101,9 @@ final class PhabricatorUserPreferences extends PhabricatorUserDAO {
     return $large;
   }
 
+  public static function filterMonospacedCSSRule($monospaced) {
+    // Prevent the user from doing dangerous things.
+    return preg_replace('/[^a-z0-9 ,".]+/i', '', $monospaced);
+  }
+
 }
diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php
index 98df4aefb7..5aafc86a37 100644
--- a/src/view/page/PhabricatorStandardPageView.php
+++ b/src/view/page/PhabricatorStandardPageView.php
@@ -287,7 +287,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       $user = $request->getUser();
       if ($user) {
         $monospaced = $user->loadPreferences()->getPreference(
-            PhabricatorUserPreferences::PREFERENCE_MONOSPACED);
+          PhabricatorUserPreferences::PREFERENCE_MONOSPACED);
       }
     }
 
@@ -295,12 +295,19 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
 
     $font_css = null;
     if (!empty($monospaced)) {
+      // We can't print this normally because escaping quotation marks will
+      // break the CSS. Instead, filter it strictly and then mark it as safe.
+      $monospaced = new PhutilSafeHTML(
+        PhabricatorUserPreferences::filterMonospacedCSSRule(
+          $monospaced));
+
       $font_css = hsprintf(
         '<style type="text/css">'.
         '.PhabricatorMonospaced, '.
         '.phabricator-remarkup .remarkup-code-block '.
           '.remarkup-code { font: %s !important; } '.
-        '</style>', $monospaced);
+        '</style>',
+        $monospaced);
     }
 
     return hsprintf(