Skip to content

Improve HTML field validation (CFM-390) #287

Merged
merged 2 commits into from Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/resources/locales/en_US/error.po
Expand Up @@ -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> tags are not allowed."

msgid "input.invalid.prefix"
msgstr "The provided value is not valid. \"{0}\" prefix is required"
Expand Down
3 changes: 3 additions & 0 deletions app/resources/locales/en_US/information.po
Expand Up @@ -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; tags are disallowed."

msgid "pagination.format"
msgstr "Page {{page}} of {{pages}}, Viewing {{start}}-{{end}} of {{count}}"

Expand Down
27 changes: 7 additions & 20 deletions app/src/Lib/Traits/ValidationTrait.php
Expand Up @@ -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.
Expand Down
24 changes: 22 additions & 2 deletions app/templates/MessageTemplates/fields.inc
Expand Up @@ -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',
Expand Down