From a616cb471d31f04a28d185aeb1bcb83637afc3cf Mon Sep 17 00:00:00 2001 From: Vladimir Gorej Date: Thu, 11 Jun 2020 14:54:40 +0200 Subject: [PATCH] fix(Markdown): render markdown in more secure way This commit changes markdown sanitization behaviour in following way: class, style and data-* attributes are removed by default. These attributes open possible vulnerability vectors to attackers. The original behavior of sanitizer (before this commit) can be enabled by *useUnsafeMarkdown* configuration option. Use this configuration option with caution and only in cases when you know what you're doing. --- docker/configurator/variables.js | 4 +++ docs/usage/configuration.md | 1 + src/core/components/array-model.jsx | 2 +- src/core/components/auth/api-key-auth.jsx | 2 +- src/core/components/auth/basic-auth.jsx | 2 +- src/core/components/auth/oauth2.jsx | 2 +- src/core/components/example.jsx | 2 +- src/core/components/headers.jsx | 4 +-- src/core/components/info.jsx | 6 ++-- src/core/components/object-model.jsx | 2 +- src/core/components/operation-tag.jsx | 2 +- src/core/components/operation.jsx | 2 +- src/core/components/parameter-row.jsx | 6 ++-- src/core/components/primitive-model.jsx | 2 +- src/core/components/providers/markdown.jsx | 25 +++++++++++++--- src/core/components/response.jsx | 2 +- .../plugins/oas3/components/callbacks.jsx | 2 +- .../plugins/oas3/components/http-auth.jsx | 2 +- .../oas3/components/operation-link.jsx | 2 +- .../plugins/oas3/components/request-body.jsx | 2 +- .../plugins/oas3/wrap-components/markdown.jsx | 12 ++++++-- test/mocha/components/markdown.jsx | 30 ++++++++++++++++--- 22 files changed, 83 insertions(+), 33 deletions(-) diff --git a/docker/configurator/variables.js b/docker/configurator/variables.js index a7aa4ff3..12d54488 100644 --- a/docker/configurator/variables.js +++ b/docker/configurator/variables.js @@ -71,6 +71,10 @@ const standardVariables = { type: "boolean", name: "showCommonExtensions" }, + USE_UNSAFE_MARKDOWN: { + type: "boolean", + name: "useUnsafeMarkdown" + }, OAUTH2_REDIRECT_URL: { type: "string", name: "oauth2RedirectUrl" diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index 38d80df3..bfe5b633 100644 --- a/docs/usage/configuration.md +++ b/docs/usage/configuration.md @@ -59,6 +59,7 @@ Parameter name | Docker variable | Description `showExtensions` | `SHOW_EXTENSIONS` | `Boolean=false`. Controls the display of vendor extension (`x-`) fields and values for Operations, Parameters, and Schema. `showCommonExtensions` | `SHOW_COMMON_EXTENSIONS` | `Boolean=false`. Controls the display of extensions (`pattern`, `maxLength`, `minLength`, `maximum`, `minimum`) fields and values for Parameters. `tagsSorter` | _Unavailable_ | `Function=(a => a)`. Apply a sort to the tag list of each API. It can be 'alpha' (sort by paths alphanumerically) or a function (see [Array.prototype.sort()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort) to learn how to write a sort function). Two tag name strings are passed to the sorter for each pass. Default is the order determined by Swagger UI. +`useUnsafeMarkdown` | `USE_UNSAFE_MARKDOWN` | `Boolean=false`. When enabled, sanitizer will leave `style`, `class` and `data-*` attributes untouched on all HTML Elements declared inside markdown strings. This parameter is **Deprecated** and will be removed in `4.0.0`. `onComplete` | _Unavailable_ | `Function=NOOP`. Provides a mechanism to be notified when Swagger UI has finished rendering a newly provided definition. ##### Network diff --git a/src/core/components/array-model.jsx b/src/core/components/array-model.jsx index cdc74166..7b6f0d72 100644 --- a/src/core/components/array-model.jsx +++ b/src/core/components/array-model.jsx @@ -25,7 +25,7 @@ export default class ArrayModel extends Component { let title = schema.get("title") || displayName || name let properties = schema.filter( ( v, key) => ["type", "items", "description", "$$ref"].indexOf(key) === -1 ) - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const ModelCollapse = getComponent("ModelCollapse") const Model = getComponent("Model") const Property = getComponent("Property") diff --git a/src/core/components/auth/api-key-auth.jsx b/src/core/components/auth/api-key-auth.jsx index e9c926b5..105e7c65 100644 --- a/src/core/components/auth/api-key-auth.jsx +++ b/src/core/components/auth/api-key-auth.jsx @@ -44,7 +44,7 @@ export default class ApiKeyAuth extends React.Component { const Row = getComponent("Row") const Col = getComponent("Col") const AuthError = getComponent("authError") - const Markdown = getComponent( "Markdown" ) + const Markdown = getComponent("Markdown", true) const JumpToPath = getComponent("JumpToPath", true) let value = this.getValue() let errors = errSelectors.allErrors().filter( err => err.get("authId") === name) diff --git a/src/core/components/auth/basic-auth.jsx b/src/core/components/auth/basic-auth.jsx index eb188715..e0ccd1fc 100644 --- a/src/core/components/auth/basic-auth.jsx +++ b/src/core/components/auth/basic-auth.jsx @@ -51,7 +51,7 @@ export default class BasicAuth extends React.Component { const Col = getComponent("Col") const AuthError = getComponent("authError") const JumpToPath = getComponent("JumpToPath", true) - const Markdown = getComponent( "Markdown" ) + const Markdown = getComponent("Markdown", true) let username = this.getValue().username let errors = errSelectors.allErrors().filter( err => err.get("authId") === name) diff --git a/src/core/components/auth/oauth2.jsx b/src/core/components/auth/oauth2.jsx index 966de0f8..efe02e4f 100644 --- a/src/core/components/auth/oauth2.jsx +++ b/src/core/components/auth/oauth2.jsx @@ -109,7 +109,7 @@ export default class Oauth2 extends React.Component { const Button = getComponent("Button") const AuthError = getComponent("authError") const JumpToPath = getComponent("JumpToPath", true) - const Markdown = getComponent( "Markdown" ) + const Markdown = getComponent("Markdown", true) const InitializedInput = getComponent("InitializedInput") const { isOAS3 } = specSelectors diff --git a/src/core/components/example.jsx b/src/core/components/example.jsx index 3b0e0737..839bcea3 100644 --- a/src/core/components/example.jsx +++ b/src/core/components/example.jsx @@ -10,7 +10,7 @@ import { stringify } from "core/utils" export default function Example(props) { const { example, showValue, getComponent } = props - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const HighlightCode = getComponent("highlightCode") if(!example) return null diff --git a/src/core/components/headers.jsx b/src/core/components/headers.jsx index c21203bc..36c94dc5 100644 --- a/src/core/components/headers.jsx +++ b/src/core/components/headers.jsx @@ -14,7 +14,7 @@ export default class Headers extends React.Component { let { headers, getComponent } = this.props const Property = getComponent("Property") - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) if ( !headers || !headers.size ) return null @@ -36,7 +36,7 @@ export default class Headers extends React.Component { if(!Im.Map.isMap(header)) { return null } - + const description = header.get("description") const type = header.getIn(["schema"]) ? header.getIn(["schema", "type"]) : header.getIn(["type"]) const schemaExample = header.getIn(["schema", "example"]) diff --git a/src/core/components/info.jsx b/src/core/components/info.jsx index a746ef6f..0ebf8811 100644 --- a/src/core/components/info.jsx +++ b/src/core/components/info.jsx @@ -61,7 +61,7 @@ class License extends React.Component { let { license, getComponent } = this.props const Link = getComponent("Link") - + let name = license.get("name") || "License" let url = license.get("url") @@ -82,7 +82,7 @@ export class InfoUrl extends React.PureComponent { getComponent: PropTypes.func.isRequired } - + render() { const { url, getComponent } = this.props @@ -112,7 +112,7 @@ export default class Info extends React.Component { let license = info.get("license") const { url:externalDocsUrl, description:externalDocsDescription } = (externalDocs || fromJS({})).toJS() - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const Link = getComponent("Link") const VersionStamp = getComponent("VersionStamp") const InfoUrl = getComponent("InfoUrl") diff --git a/src/core/components/object-model.jsx b/src/core/components/object-model.jsx index 324b4355..d52fcb0c 100644 --- a/src/core/components/object-model.jsx +++ b/src/core/components/object-model.jsx @@ -40,7 +40,7 @@ export default class ObjectModel extends Component { let requiredProperties = schema.get("required") const JumpToPath = getComponent("JumpToPath", true) - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const Model = getComponent("Model") const ModelCollapse = getComponent("ModelCollapse") diff --git a/src/core/components/operation-tag.jsx b/src/core/components/operation-tag.jsx index dddf6b02..d2e3f31d 100644 --- a/src/core/components/operation-tag.jsx +++ b/src/core/components/operation-tag.jsx @@ -44,7 +44,7 @@ export default class OperationTag extends React.Component { const isDeepLinkingEnabled = deepLinking && deepLinking !== "false" const Collapse = getComponent("Collapse") - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const DeepLink = getComponent("DeepLink") const Link = getComponent("Link") diff --git a/src/core/components/operation.jsx b/src/core/components/operation.jsx index cf52bb7b..29130be2 100644 --- a/src/core/components/operation.jsx +++ b/src/core/components/operation.jsx @@ -93,7 +93,7 @@ export default class Operation extends PureComponent { const Execute = getComponent( "execute" ) const Clear = getComponent( "clear" ) const Collapse = getComponent( "Collapse" ) - const Markdown = getComponent( "Markdown" ) + const Markdown = getComponent("Markdown", true) const Schemes = getComponent( "schemes" ) const OperationServers = getComponent( "OperationServers" ) const OperationExt = getComponent( "OperationExt" ) diff --git a/src/core/components/parameter-row.jsx b/src/core/components/parameter-row.jsx index 41b5ab44..59fe8954 100644 --- a/src/core/components/parameter-row.jsx +++ b/src/core/components/parameter-row.jsx @@ -102,7 +102,7 @@ export default class ParameterRow extends Component { .get("content", Map()) .keySeq() .first() - + // getSampleSchema could return null const generatedSampleValue = schema ? getSampleSchema(schema.toJS(), parameterMediaType, { includeWriteOnly: true @@ -144,7 +144,7 @@ export default class ParameterRow extends Component { this.onChangeWrapper(initialValue) } else if( schema && schema.get("type") === "object" - && generatedSampleValue + && generatedSampleValue && !paramWithMeta.get("examples") ) { // Object parameters get special treatment.. if the user doesn't set any @@ -202,7 +202,7 @@ export default class ParameterRow extends Component { /> const ModelExample = getComponent("modelExample") - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const ParameterExt = getComponent("ParameterExt") const ParameterIncludeEmpty = getComponent("ParameterIncludeEmpty") const ExamplesSelectValueRetainer = getComponent("ExamplesSelectValueRetainer") diff --git a/src/core/components/primitive-model.jsx b/src/core/components/primitive-model.jsx index 6ddadbd8..22442daa 100644 --- a/src/core/components/primitive-model.jsx +++ b/src/core/components/primitive-model.jsx @@ -34,7 +34,7 @@ export default class Primitive extends Component { let properties = schema .filter( ( v, key) => ["enum", "type", "format", "description", "$$ref"].indexOf(key) === -1 ) .filterNot( (v, key) => extensions.has(key) ) - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const EnumModel = getComponent("EnumModel") const Property = getComponent("Property") diff --git a/src/core/components/providers/markdown.jsx b/src/core/components/providers/markdown.jsx index dae1f186..73f835cf 100644 --- a/src/core/components/providers/markdown.jsx +++ b/src/core/components/providers/markdown.jsx @@ -16,7 +16,7 @@ DomPurify.addHook("beforeSanitizeElements", function (current, ) { return current }) -function Markdown({ source, className = "" }) { +function Markdown({ source, className = "", getConfigs }) { if (typeof source !== "string") { return null } @@ -30,8 +30,9 @@ function Markdown({ source, className = "" }) { md.core.ruler.disable(["replacements", "smartquotes"]) + const { useUnsafeMarkdown } = getConfigs() const html = md.render(source) - const sanitized = sanitizer(html) + const sanitized = sanitizer(html, { useUnsafeMarkdown }) if (!source || !html || !sanitized) { return null @@ -44,14 +45,30 @@ function Markdown({ source, className = "" }) { Markdown.propTypes = { source: PropTypes.string.isRequired, - className: PropTypes.string + className: PropTypes.string, + getConfigs: PropTypes.func, +} + +Markdown.defaultProps = { + getConfigs: () => ({ useUnsafeMarkdown: false }), } export default Markdown -export function sanitizer(str) { +export function sanitizer(str, { useUnsafeMarkdown = false } = {}) { + const ALLOW_DATA_ATTR = useUnsafeMarkdown + const FORBID_ATTR = useUnsafeMarkdown ? [] : ["style", "class"] + + if (useUnsafeMarkdown && !sanitizer.hasWarnedAboutDeprecation) { + console.warn(`useUnsafeMarkdown display configuration parameter is deprecated since >3.26.0 and will be removed in v4.0.0.`) + sanitizer.hasWarnedAboutDeprecation = true + } + return DomPurify.sanitize(str, { ADD_ATTR: ["target"], FORBID_TAGS: ["style"], + ALLOW_DATA_ATTR, + FORBID_ATTR, }) } +sanitizer.hasWarnedAboutDeprecation = false diff --git a/src/core/components/response.jsx b/src/core/components/response.jsx index 3a98046d..e3801290 100644 --- a/src/core/components/response.jsx +++ b/src/core/components/response.jsx @@ -93,7 +93,7 @@ export default class Response extends React.Component { const Headers = getComponent("headers") const HighlightCode = getComponent("highlightCode") const ModelExample = getComponent("modelExample") - const Markdown = getComponent( "Markdown" ) + const Markdown = getComponent("Markdown", true) const OperationLink = getComponent("operationLink") const ContentType = getComponent("contentType") const ExamplesSelect = getComponent("ExamplesSelect") diff --git a/src/core/plugins/oas3/components/callbacks.jsx b/src/core/plugins/oas3/components/callbacks.jsx index 56a8fd18..e03319c9 100644 --- a/src/core/plugins/oas3/components/callbacks.jsx +++ b/src/core/plugins/oas3/components/callbacks.jsx @@ -5,7 +5,7 @@ import { fromJS } from "immutable" const Callbacks = (props) => { let { callbacks, getComponent, specPath } = props - // const Markdown = getComponent("Markdown") + // const Markdown = getComponent("Markdown", true) const OperationContainer = getComponent("OperationContainer", true) if(!callbacks) { diff --git a/src/core/plugins/oas3/components/http-auth.jsx b/src/core/plugins/oas3/components/http-auth.jsx index 5c324af0..00f5ac2c 100644 --- a/src/core/plugins/oas3/components/http-auth.jsx +++ b/src/core/plugins/oas3/components/http-auth.jsx @@ -51,7 +51,7 @@ export default class HttpAuth extends React.Component { const Row = getComponent("Row") const Col = getComponent("Col") const AuthError = getComponent("authError") - const Markdown = getComponent( "Markdown" ) + const Markdown = getComponent("Markdown", true) const JumpToPath = getComponent("JumpToPath", true) const scheme = (schema.get("scheme") || "").toLowerCase() diff --git a/src/core/plugins/oas3/components/operation-link.jsx b/src/core/plugins/oas3/components/operation-link.jsx index 29342872..086e145f 100644 --- a/src/core/plugins/oas3/components/operation-link.jsx +++ b/src/core/plugins/oas3/components/operation-link.jsx @@ -6,7 +6,7 @@ class OperationLink extends Component { render() { const { link, name, getComponent } = this.props - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) let targetOp = link.get("operationId") || link.get("operationRef") let parameters = link.get("parameters") && link.get("parameters").toJS() diff --git a/src/core/plugins/oas3/components/request-body.jsx b/src/core/plugins/oas3/components/request-body.jsx index 5e5d7a83..c02a07e8 100644 --- a/src/core/plugins/oas3/components/request-body.jsx +++ b/src/core/plugins/oas3/components/request-body.jsx @@ -54,7 +54,7 @@ const RequestBody = ({ onChange(e.target.files[0]) } - const Markdown = getComponent("Markdown") + const Markdown = getComponent("Markdown", true) const ModelExample = getComponent("modelExample") const RequestBodyEditor = getComponent("RequestBodyEditor") const HighlightCode = getComponent("highlightCode") diff --git a/src/core/plugins/oas3/wrap-components/markdown.jsx b/src/core/plugins/oas3/wrap-components/markdown.jsx index 81b2f1b6..a7510e72 100644 --- a/src/core/plugins/oas3/wrap-components/markdown.jsx +++ b/src/core/plugins/oas3/wrap-components/markdown.jsx @@ -9,14 +9,15 @@ const parser = new Remarkable("commonmark") parser.block.ruler.enable(["table"]) parser.set({ linkTarget: "_blank" }) -export const Markdown = ({ source, className = "" }) => { +export const Markdown = ({ source, className = "", getConfigs }) => { if(typeof source !== "string") { return null } - + if ( source ) { + const { useUnsafeMarkdown } = getConfigs() const html = parser.render(source) - const sanitized = sanitizer(html) + const sanitized = sanitizer(html, { useUnsafeMarkdown }) let trimmed @@ -38,6 +39,11 @@ export const Markdown = ({ source, className = "" }) => { Markdown.propTypes = { source: PropTypes.string, className: PropTypes.string, + getConfigs: PropTypes.func, +} + +Markdown.defaultProps = { + getConfigs: () => ({ useUnsafeMarkdown: false }), } export default OAS3ComponentWrapFactory(Markdown) diff --git a/test/mocha/components/markdown.jsx b/test/mocha/components/markdown.jsx index a89003b3..8a9b71d2 100644 --- a/test/mocha/components/markdown.jsx +++ b/test/mocha/components/markdown.jsx @@ -7,10 +7,18 @@ import { Markdown as OAS3Markdown } from "corePlugins/oas3/wrap-components/markd describe("Markdown component", function() { describe("Swagger 2.0", function() { - it("allows span elements with class attrib", function() { - const str = `ONE` - const el = render() - expect(el.html()).toEqual(`

ONE

\n
`) + it("allows elements with class, style and data-* attribs", function() { + const getConfigs = () => ({ useUnsafeMarkdown: true }) + const str = `ONE` + const el = render() + expect(el.html()).toEqual(`

ONE

\n
`) + }) + + it("strips class, style and data-* attribs from elements", function() { + const getConfigs = () => ({ useUnsafeMarkdown: false }) + const str = `ONE` + const el = render() + expect(el.html()).toEqual(`

ONE

\n
`) }) it("allows td elements with colspan attrib", function() { @@ -57,6 +65,20 @@ describe("Markdown component", function() { }) describe("OAS 3", function() { + it("allows elements with class, style and data-* attribs", function() { + const getConfigs = () => ({ useUnsafeMarkdown: true }) + const str = `ONE` + const el = render() + expect(el.html()).toEqual(`

ONE

`) + }) + + it("strips class, style and data-* attribs from elements", function() { + const getConfigs = () => ({ useUnsafeMarkdown: false }) + const str = `ONE` + const el = render() + expect(el.html()).toEqual(`

ONE

`) + }) + it("allows image elements", function() { const str = `![Image alt text](http://image.source "Image title")` const el = render()