Improve HTML field validation (CFM-390)

COmanage · Feb 10, 2025 · 3ea5a9e · 3ea5a9e
1 parent 1fa47b4
commit 3ea5a9e
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 23 deletions.
diff --git a/app/resources/locales/en_US/error.po b/app/resources/locales/en_US/error.po
@@ -197,7 +197,7 @@ msgid "input.invalid.email"
 msgstr "The provided value is not a valid email address"
 
 msgid "input.invalid.html"
-msgstr "HTML should be well-formed, and non-standard attributes, <script>, and <style> tags are not allowed."
+msgstr "<script> and <style> tags are not allowed."
 
 msgid "input.invalid.prefix"
 msgstr "The provided value is not valid. \"{0}\" prefix is required"

diff --git a/app/resources/locales/en_US/information.po b/app/resources/locales/en_US/information.po
@@ -111,6 +111,9 @@ msgstr "Visit link"
 msgid "HistoryRecords.xref"
 msgstr "Additional History Records may be available via Petitions and Provisioning Status"
 
+msgid "html.sanitization"
+msgstr "HTML input will be sanitized on output, and &lt;script&gt; and &lt;style&gt; tags are disallowed."
+
 msgid "pagination.format"
 msgstr "Page {{page}} of {{pages}}, Viewing {{start}}-{{end}} of {{count}}"
 

diff --git a/app/src/Lib/Traits/ValidationTrait.php b/app/src/Lib/Traits/ValidationTrait.php
@@ -240,28 +240,15 @@ public function validateInput($value, array $context) {
     if(!empty($context['type'])) {
       switch($context['type']) {
         case 'html':
-          // We are accepting HTML input. Pass it through the Symfony HTML Sanitizer to
-          // disallow dom elements like <script> and <style>.
-          $htmlSanitizer = new HtmlSanitizer(
-            // Allow all elements from the W3C Sanitizer API. This is more permissive than "allowSafeElements()".
-            // See: https://github.com/symfony/symfony/blob/7.2/src/Symfony/Component/HtmlSanitizer/Reference/W3CReference.php
-            (new HtmlSanitizerConfig())->allowStaticElements()
-          );
-          $sanitizedValue = $htmlSanitizer->sanitize($value);
-
-          // Compare $value and $sanitizedValue to see if anything changed. Because white space and closing slashes
-          // can be significantly altered during sanitization, normalize the strings prior to comparison.
-          // (Unfortunately, the HtmlSanitizer does not generate a report on what it changed, which would be better.)
-          $valueNormalized = preg_replace(['/\s+/','/\//'], '', $value);
-          $sanitizedValueNormalized = preg_replace(['/\s+/','/\//'], '', $sanitizedValue);
-          // XXX Note: stripping forward slashes allows us to ignore the differences between <br> and <br/>
-          // (for example), but it also allows malformed tags such as <br////> or <div/></div> to get through.
-
-          if($valueNormalized !== $sanitizedValueNormalized) {
-            // Disallowed HTML is in the input, so throw an error.
+          // We are accepting HTML input. We will mostly pass it all through and ensure
+          // properly sanitized output. However, to help warn users about entering tags that 
+          // will be stripped, we can do some very rudimentary checking for script and 
+          // style tags. (An informational note should be placed below these fields as well.)
+          $lowercaseVal = strtolower($value);
+          if(str_contains($lowercaseVal, '<script') || str_contains($lowercaseVal, '<style')) {
+            // Disallowed HTML is in the input, so warn the user.
             return __d('error', 'input.invalid.html');
           }
-
           return true;
         default:
           // We use h() (htmlspecialchars) for consistency with the views.

diff --git a/app/templates/MessageTemplates/fields.inc b/app/templates/MessageTemplates/fields.inc
@@ -74,8 +74,28 @@ if($vv_action == 'add' || $vv_action == 'edit') {
 
   foreach ([
              'subject',
-             'body_text',
-             'body_html',
+             'body_text'
+           ] as $field) {
+    print $this->element('form/listItem', [
+      'arguments' => [
+        'fieldName' => $field
+      ]]);
+  }
+
+  $afterField = 
+    '<div class="alert alert-info small p-2 d-flex gap-1" role="alert">' .
+    '  <em class="material-symbols-outlined" aria-hidden="true">info</em>' .  
+    '  <span class="alert-text">' . __d('information','html.sanitization') . '</span>' .
+    '</div>';
+
+  print $this->element('form/listItem', [
+    'arguments' => [
+      'fieldName' => 'body_html'
+    ],
+    'afterField' => $afterField
+  ]);
+
+  foreach ([
              'cc',
              'bcc',
              'reply_to',