From 2e27394be3e6d23eeac5fd79e08fb38a700f0b7a Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Fri, 21 Apr 2023 15:38:39 -0500 Subject: [PATCH] Backport UI: Remove usage of htmlSafe (#20235) (#20262) * UI: Remove usage of htmlSafe (#20235) * Fix test helper import --- changelog/20235.txt | 3 ++ ui/app/components/wizard-content.js | 9 +++-- .../components/wizard/features-selection.js | 5 ++- .../components/diff-version-selector.hbs | 3 +- .../templates/components/wizard-progress.hbs | 2 +- ui/lib/core/addon/components/confirm.js | 3 +- .../addon/components/replication-dashboard.js | 7 ++-- ui/lib/core/addon/helpers/sanitized-html.js | 14 ++++++++ .../components/replication-dashboard.hbs | 2 +- ui/lib/core/app/helpers/sanitized-html.js | 1 + ui/lib/core/package.json | 1 + ui/package.json | 1 + .../helpers/sanitized-html-test.js | 35 +++++++++++++++++++ ui/yarn.lock | 5 +++ 14 files changed, 72 insertions(+), 19 deletions(-) create mode 100644 changelog/20235.txt create mode 100644 ui/lib/core/addon/helpers/sanitized-html.js create mode 100644 ui/lib/core/app/helpers/sanitized-html.js create mode 100644 ui/tests/integration/helpers/sanitized-html-test.js diff --git a/changelog/20235.txt b/changelog/20235.txt new file mode 100644 index 000000000000..d1b9f8a6e923 --- /dev/null +++ b/changelog/20235.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: remove use of htmlSafe except when first sanitized +``` diff --git a/ui/app/components/wizard-content.js b/ui/app/components/wizard-content.js index 98ec13b33bb5..ecba2d7efc2f 100644 --- a/ui/app/components/wizard-content.js +++ b/ui/app/components/wizard-content.js @@ -3,7 +3,6 @@ import { inject as service } from '@ember/service'; import Component from '@ember/component'; import { computed } from '@ember/object'; import { FEATURE_MACHINE_STEPS, INIT_STEPS } from 'vault/helpers/wizard-constants'; -import { htmlSafe } from '@ember/template'; export default Component.extend({ wizard: service(), @@ -84,25 +83,25 @@ export default Component.extend({ let bar = []; if (this.currentTutorialProgress) { bar.push({ - style: htmlSafe(`width:${this.currentTutorialProgress.percentage}%;`), + style: `width:${this.currentTutorialProgress.percentage}%;`, completed: false, showIcon: true, }); } else { if (this.currentFeatureProgress) { this.completedFeatures.forEach((feature) => { - bar.push({ style: htmlSafe('width:100%;'), completed: true, feature: feature, showIcon: true }); + bar.push({ style: 'width:100%;', completed: true, feature: feature, showIcon: true }); }); this.wizard.featureList.forEach((feature) => { if (feature === this.currentMachine) { bar.push({ - style: htmlSafe(`width:${this.currentFeatureProgress.percentage}%;`), + style: `width:${this.currentFeatureProgress.percentage}%;`, completed: this.currentFeatureProgress.percentage == 100 ? true : false, feature: feature, showIcon: true, }); } else { - bar.push({ style: htmlSafe('width:0%;'), completed: false, feature: feature, showIcon: true }); + bar.push({ style: 'width:0%;', completed: false, feature: feature, showIcon: true }); } }); } diff --git a/ui/app/components/wizard/features-selection.js b/ui/app/components/wizard/features-selection.js index 4f7a528faa33..669e6a2f93d6 100644 --- a/ui/app/components/wizard/features-selection.js +++ b/ui/app/components/wizard/features-selection.js @@ -3,7 +3,6 @@ import { inject as service } from '@ember/service'; import Component from '@ember/component'; import { computed } from '@ember/object'; import { FEATURE_MACHINE_TIME } from 'vault/helpers/wizard-constants'; -import { htmlSafe } from '@ember/template'; export default Component.extend({ wizard: service(), @@ -50,10 +49,10 @@ export default Component.extend({ }), selectProgress: computed('selectedFeatures', function () { let bar = this.selectedFeatures.map((feature) => { - return { style: htmlSafe('width:0%;'), completed: false, showIcon: true, feature: feature }; + return { style: 'width:0%;', completed: false, showIcon: true, feature: feature }; }); if (bar.length === 0) { - bar = [{ style: htmlSafe('width:0%;'), showIcon: false }]; + bar = [{ style: 'width:0%;', showIcon: false }]; } return bar; }), diff --git a/ui/app/templates/components/diff-version-selector.hbs b/ui/app/templates/components/diff-version-selector.hbs index ccc3127e7b70..50936aff4f1b 100644 --- a/ui/app/templates/components/diff-version-selector.hbs +++ b/ui/app/templates/components/diff-version-selector.hbs @@ -98,6 +98,5 @@
- {{! template-lint-configure no-triple-curlies "warn" }} - 
{{{this.visualDiff}}}
+ 
{{sanitized-html this.visualDiff}}
\ No newline at end of file diff --git a/ui/app/templates/components/wizard-progress.hbs b/ui/app/templates/components/wizard-progress.hbs index 6bae1819f966..29899c5db81e 100644 --- a/ui/app/templates/components/wizard-progress.hbs +++ b/ui/app/templates/components/wizard-progress.hbs @@ -2,7 +2,7 @@ {{#each @progressBar as |bar|}}
- + {{#if bar.showIcon}} diff --git a/ui/lib/core/addon/components/confirm.js b/ui/lib/core/addon/components/confirm.js index 0c568c60c530..877eb03c8669 100644 --- a/ui/lib/core/addon/components/confirm.js +++ b/ui/lib/core/addon/components/confirm.js @@ -1,6 +1,5 @@ import Component from '@ember/component'; import { computed } from '@ember/object'; -import { htmlSafe } from '@ember/template'; import layout from '../templates/components/confirm'; import { next } from '@ember/runloop'; @@ -29,7 +28,7 @@ export default Component.extend({ height: 0, focusTrigger: null, style: computed('height', function () { - return htmlSafe(`height: ${this.height}px`); + return `height: ${this.height}px`; }), wormholeReference: null, wormholeId: computed('elementId', function () { diff --git a/ui/lib/core/addon/components/replication-dashboard.js b/ui/lib/core/addon/components/replication-dashboard.js index 89e9a8c21e50..ddb1a4ae837b 100644 --- a/ui/lib/core/addon/components/replication-dashboard.js +++ b/ui/lib/core/addon/components/replication-dashboard.js @@ -2,7 +2,6 @@ import Component from '@ember/component'; import { computed } from '@ember/object'; import { clusterStates } from 'core/helpers/cluster-states'; import { capitalize } from '@ember/string'; -import { htmlSafe } from '@ember/template'; import layout from '../templates/components/replication-dashboard'; /** @@ -30,7 +29,7 @@ import layout from '../templates/components/replication-dashboard'; * @param {Boolean} [isSummaryDashboard=false] - Only true when the cluster is both a dr and performance primary. If true, replicationDetailsSummary is populated and used to pass through the cluster details. * @param {Object} replicationDetailsSummary=null - An Ember data object computed off the Ember Model. It combines the Model.dr and Model.performance objects into one and contains details specific to the mode replication. * @param {Object} replicationDetails=null - An Ember data object pulled from the Ember Model. It contains details specific to the whether the replication is dr or performance. - * @param {String} clusterMode=null - The cluster mode passed through to a table component. + * @param {String} clusterMode=null - The cluster mode passed through to a table component. * @param {Object} reindexingDetails=null - An Ember data object used to show a reindexing progress bar. */ @@ -89,9 +88,7 @@ export default Component.extend({ }), reindexMessage: computed('isSecondary', 'progressBar', function () { if (!this.isSecondary) { - return htmlSafe( - 'This can cause a delay depending on the size of the data store. You can not use Vault during this time.' - ); + return 'This can cause a delay depending on the size of the data store. You can not use Vault during this time.'; } return 'This can cause a delay depending on the size of the data store. You can use Vault during this time.'; }), diff --git a/ui/lib/core/addon/helpers/sanitized-html.js b/ui/lib/core/addon/helpers/sanitized-html.js new file mode 100644 index 000000000000..cfa99d723990 --- /dev/null +++ b/ui/lib/core/addon/helpers/sanitized-html.js @@ -0,0 +1,14 @@ +import { helper } from '@ember/component/helper'; +import { debug } from '@ember/debug'; +import { htmlSafe } from '@ember/template'; +import { sanitize } from 'dompurify'; + +export default helper(function sanitizedHtml([htmlString]) { + try { + return htmlSafe(sanitize(htmlString)); + } catch (e) { + debug('Error sanitizing string', e); + // I couldn't get this to actually fail but as a fallback render the value as-is + return htmlString; + } +}); diff --git a/ui/lib/core/addon/templates/components/replication-dashboard.hbs b/ui/lib/core/addon/templates/components/replication-dashboard.hbs index 307ebff2b755..9b5f5741d931 100644 --- a/ui/lib/core/addon/templates/components/replication-dashboard.hbs +++ b/ui/lib/core/addon/templates/components/replication-dashboard.hbs @@ -5,7 +5,7 @@ @title={{concat "Re-indexing in progress" this.reindexingStage}} @type="info" @progressBar={{this.progressBar}} - @message={{this.reindexMessage}} + @message={{sanitized-html this.reindexMessage}} data-test-isReindexing />
diff --git a/ui/lib/core/app/helpers/sanitized-html.js b/ui/lib/core/app/helpers/sanitized-html.js new file mode 100644 index 000000000000..7bd6cb8d9944 --- /dev/null +++ b/ui/lib/core/app/helpers/sanitized-html.js @@ -0,0 +1 @@ +export { default } from 'core/helpers/sanitized-html'; diff --git a/ui/lib/core/package.json b/ui/lib/core/package.json index 3aa8729d252e..357d85e3c6f6 100644 --- a/ui/lib/core/package.json +++ b/ui/lib/core/package.json @@ -8,6 +8,7 @@ "date-fns": "*", "@icholy/duration": "*", "base64-js": "*", + "dompurify": "*", "ember-auto-import": "*", "ember-basic-dropdown": "*", "ember-cli-babel": "*", diff --git a/ui/package.json b/ui/package.json index 76efdacdf865..a2ea22bdd92a 100644 --- a/ui/package.json +++ b/ui/package.json @@ -84,6 +84,7 @@ "date-fns-tz": "^1.2.2", "deepmerge": "^4.0.0", "doctoc": "^1.4.0", + "dompurify": "^3.0.2", "ember-api-actions": "^0.2.8", "ember-auto-import": "^1.12.0", "ember-basic-dropdown": "4.0.3", diff --git a/ui/tests/integration/helpers/sanitized-html-test.js b/ui/tests/integration/helpers/sanitized-html-test.js new file mode 100644 index 000000000000..b677f2a92dd1 --- /dev/null +++ b/ui/tests/integration/helpers/sanitized-html-test.js @@ -0,0 +1,35 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; + +module('Integration | Helper | sanitized-html', function (hooks) { + setupRenderingTest(hooks); + + test('it does not alter if string is safe', async function (assert) { + this.set('inputValue', 'height: 15.33px'); + + await render(hbs`{{sanitized-html this.inputValue}}`); + assert.dom(this.element).hasText('height: 15.33px'); + }); + + test('it strips unsafe HTML before rendering safe HTML', async function (assert) { + this.set( + 'inputValue', + '
This is something
' + ); + + await render(hbs`{{sanitized-html this.inputValue}}`); + assert.dom('[data-test-thing]').hasTagName('main'); + assert.dom('[data-test-thing]').hasText('This is something', 'preserves non-problematic content'); + assert.dom('[data-test-script]').doesNotExist('Script is stripped from render'); + }); + + test('it does not invoke functions passed as value', async function (assert) { + this.set('inputValue', () => { + window.alert('h4cK3d'); + }); + await render(hbs`{{sanitized-html this.inputValue}}`); + assert.dom(this.element).hasText("() => { window.alert('h4cK3d'); }"); + }); +}); diff --git a/ui/yarn.lock b/ui/yarn.lock index 5d4b8801096d..f575c34eb940 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -7104,6 +7104,11 @@ domhandler@^2.3.0: dependencies: domelementtype "1" +dompurify@^3.0.2: + version "3.0.2" + resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-3.0.2.tgz#bc4c7c011c825e7704341a285461d8d407d9429a" + integrity sha512-B8c6JdiEpxAKnd8Dm++QQxJL4lfuc757scZtcapj6qjTjrQzyq5iAyznLKVvK+77eYNiFblHBlt7MM0fOeqoKw== + domutils@1.5.1: version "1.5.1" resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.5.1.tgz#dcd8488a26f563d61079e48c9f7b7e32373682cf"