From: Coleman Watts <coleman@civicrm.org>
Date: Thu, 28 May 2020 19:15:26 +0000 (-0400)
Subject: CKEditor Config - Validate input before saving config file
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=1ea6de73b8c7a2aacdc6b3573231cdac93038348;p=civicrm-core.git

CKEditor Config - Validate input before saving config file

Also removes support for 'customConfig' supplimental file.
---

diff --git a/CRM/Admin/Form/CKEditorConfig.php b/CRM/Admin/Form/CKEditorConfig.php
index 92180dbb4c..fd1e5c8ba9 100644
--- a/CRM/Admin/Form/CKEditorConfig.php
+++ b/CRM/Admin/Form/CKEditorConfig.php
@@ -33,6 +33,7 @@ class CRM_Admin_Form_CKEditorConfig extends CRM_Core_Form {
     'extraPlugins',
     'toolbarGroups',
     'removeButtons',
+    'customConfig',
     'filebrowserBrowseUrl',
     'filebrowserImageBrowseUrl',
     'filebrowserFlashBrowseUrl',
@@ -105,23 +106,31 @@ class CRM_Admin_Form_CKEditorConfig extends CRM_Core_Form {
       // Standardize line-endings
       . preg_replace('~\R~u', "\n", $params['config']);
 
-    // Use all params starting with config_
+    // Generate a whitelist of allowed config params
+    $allOptions = json_decode(file_get_contents(\Civi::paths()->getPath('[civicrm.root]/js/wysiwyg/ck-options.json')), TRUE);
+    // These two aren't really blacklisted they're just in a different part of the form
+    $blackList = array_diff($this->blackList, ['skin', 'extraPlugins']);
+    // All options minus blacklist = whitelist
+    $whiteList = array_diff(array_column($allOptions, 'id'), $blackList);
+
+    // Save whitelisted params starting with config_
     foreach ($params as $key => $val) {
       $val = trim($val);
-      if (strpos($key, 'config_') === 0 && strlen($val)) {
+      if (strpos($key, 'config_') === 0 && strlen($val) && in_array(substr($key, 7), $whiteList)) {
         if ($val != 'true' && $val != 'false' && $val != 'null' && $val[0] != '{' && $val[0] != '[' && !is_numeric($val)) {
-          $val = json_encode($val, JSON_UNESCAPED_SLASHES);
+          $val = '"' . $val . '"';
         }
-        elseif ($val[0] == '{' || $val[0] == '[') {
-          if (!is_array(json_decode($val, TRUE))) {
-            // Invalid JSON. Do not save.
-            continue;
+        try {
+          $val = CRM_Utils_JS::encode(CRM_Utils_JS::decode($val, TRUE));
+          $pos = strrpos($config, '};');
+          $key = preg_replace('/^config_/', 'config.', $key);
+          $setting = "\n\t{$key} = {$val};\n";
+          $config = substr_replace($config, $setting, $pos, 0);
+        } catch (CRM_Core_Exception $e) {
+          if (!empty($params['save'])) {
+            CRM_Core_Session::setStatus(ts("Error saving %1.", [1 => $key]), ts('Invalid Value'), 'error');
           }
         }
-        $pos = strrpos($config, '};');
-        $key = preg_replace('/^config_/', 'config.', $key);
-        $setting = "\n\t{$key} = {$val};\n";
-        $config = substr_replace($config, $setting, $pos, 0);
       }
     }
     self::saveConfigFile($this->preset, $config);
diff --git a/js/wysiwyg/admin.ckeditor-configurator.js b/js/wysiwyg/admin.ckeditor-configurator.js
index 00d1372e6a..f46c48fce9 100644
--- a/js/wysiwyg/admin.ckeditor-configurator.js
+++ b/js/wysiwyg/admin.ckeditor-configurator.js
@@ -65,15 +65,7 @@
   }
 
   function validateJson() {
-    var val = $(this).val();
-    $(this).parent().removeClass('crm-error');
-    if (val[0] === '[' || val[0] === '{') {
-      try {
-        JSON.parse(val);
-      } catch (e) {
-        $(this).parent().addClass('crm-error');
-      }
-    }
+    // TODO: strict json isn't required so we can't use JSON.parse for error checking. Need something like angular.eval.
   }
 
   function addOption() {