[Snyk: High] Arbitrary Code Execution in sanitize-html (due 10/9/20)

[lbeaufort]

Vulnerable module: sanitize-html

Introduced through: sanitize-html@1.18.4
No known exploit
Fixed in: 2.0.0-beta

Detailed paths
Introduced through: fec-cms@1.0.0 › sanitize-html@1.18.4
Remediation: No remediation path available.

Overview
sanitize-html is a library that allows you to clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis

Affected versions of this package are vulnerable to Arbitrary Code Execution. Tag transformations which turn an attribute value into a text node using transformTags could be vulnerable to code execution.

Completion criteria:

• Verify that this is an issue and if it is take appropriate to address the vulnerability

[johnnyporkchops]

html-sanitize is used in helpers.js to build the sanitizeValue() function:

• fec-cms/fec/fec/static/js/modules/helpers.js

  Line 514 in 393f85f

  function sanitizeValue(value) {

This is currently used in typeahead.js here:

• fec-cms/fec/fec/static/js/modules/typeahead.js

  Line 199 in 393f85f

  id: helpers.sanitizeValue(query),

• fec-cms/fec/fec/static/js/modules/typeahead.js

  Line 223 in 393f85f

  id: helpers.sanitizeValue(query),

And. in filter-typeahead.js:

• fec-cms/fec/fec/static/js/modules/filters/filter-typeahead.js

  Line 37 in 393f85f

  syncResults([{ id: helpers.sanitizeValue(query) }]);

While the Snyk alert says "No remediation paths available", it says "Fixed in Beta 2.0". Clicking through to "More about this issue" link, then confusingly says "Remediation Upgrade sanitize-html to version 2.0.0-beta or higher"

Attempts to upgrade to the 2.0 release candidate (or any of the 2.0 Beta versions)fails JS tests.

There are other possible alternatives which I was unsuccessful in using as a replacement, but need more research on their usage:
https://github.com/cure53/DOMPurify
-For DOMPurify We need to figure out how to require and use them in the scripts. DOMPurify is not as straightforward as requiring as a variable and replacing html-sanitize.
https://github.com/yahoo/xss-filters
-To use xss-filters requires that we choose the filter we want to use along with the require variable (i.e: xssFilters.inHTMLData(value)

[johnnyporkchops]

SInce the above comment, version 2.0.0 of sanitize-html has been released and Snyk remediation path has been updated to Upgrade to sanitize-html@2.0.0. However, this upgrade still fails test related to postcss module
Upgrade PR: #4069

[johnnyporkchops]

In package-.lock.json, we have postcss 8.0.9, sanitize-html has 8.0.2 listed there as requirement. Tried downgrading to 8.0.2 locally and get same errors.

[dorothyyeager]

No remediation path at this time.

[jason-upchurch]

Hi @johnnyporkchops, I was looking at other vulnerabilities in fec-cms and saw this changelog from sanitize-html in case it's useful at all:

Backwards compatibility breaks:
There is no build. You should no longer directly link to a sanitize-html file directly in the browser as it is using modern Javascript that is not fully supported by all major browsers (depending on your definition). You should now include sanitize-html in your project build for this purpose if you have one.
On the server side, Node.js 10 or higher is required.
The default allowedTags array was updated significantly. This mostly added HTML tags to be more comprehensive by default. You should review your projects and consider the allowedTags defaults if you are not already overriding them.

(ref: https://github.com/apostrophecms/sanitize-html/blob/main/CHANGELOG.md)

[johnnyporkchops]

The recommended remediation path , upgrade to 2.0.0, now fails upon npm run build, whereas before it was only failing pytest.
Still no remediation path available.

Error related to postcss, research has been unfruitful in targeting the error related to this module:

ERROR in ../node_modules/postcss/lib/lazy-result.js
    Module parse failed: Unexpected token (107:21)
    You may need an appropriate loader to handle this file type.
    | 
    |     this.result = new Result(processor, root, opts)
    |     this.helpers = { ...postcss, result: this.result, postcss }
    |     this.plugins = this.processor.plugins.map(plugin => {
    |       if (typeof plugin === 'object' && plugin.prepare) {
     @ ../node_modules/postcss/lib/postcss.js 5:17-41
     @ ../node_modules/sanitize-html/index.js
     @ ./fec/static/js/modules/helpers.js
     @ ./fec/static/js/widgets/aggregate-totals-box.js
    
    ERROR in ../node_modules/postcss/lib/declaration.js
    Module parse failed: Unexpected token (12:19)
    You may need an appropriate loader to handle this file type.
    |       typeof defaults.value !== 'string'
    |     ) {
    |       defaults = { ...defaults, value: String(defaults.value) }
    |     }
    |     super(defaults)
     @ ../node_modules/postcss/lib/postcss.js 4:18-42
     @ ../node_modules/sanitize-html/index.js
     @ ./fec/static/js/modules/helpers.js
     @ ./fec/static/js/widgets/aggregate-totals-box.js

[rfultz]

Moving to Blocked since this is a lower risk vulnerability for us and has so many other hurdles

[lbeaufort]

We removed sanitize-html with this PR: #4500