search

fecgov / fec-cms

The content management system (CMS) for the new Federal Election Commission website.
https://www.fec.gov
Other
98 stars 40 forks source link

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

Closed lbeaufort closed 3 years ago

lbeaufort commented 4 years ago

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:

johnnyporkchops commented 4 years ago

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

This is currently used in typeahead.js here:

And. in filter-typeahead.js:

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 commented 4 years ago

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: https://github.com/fecgov/fec-cms/pull/4069

johnnyporkchops commented 4 years ago

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 commented 4 years ago

No remediation path at this time.

jason-upchurch commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

Looks like postcss-loader requires Webpack 4, which we've had stalled for a while #2708

rfultz commented 3 years ago

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

lbeaufort commented 3 years ago

We removed sanitize-html with this PR: https://github.com/fecgov/fec-cms/pull/4500