fix: anchor tag safety (via #4789)

* v3.17.6

* release(3.17.6): rebuild dist

* add failing tests

* fix Link component

* fix OnlineValidatorBadge component

* switch from <a> to <Link> in operation components

* make Markdown inputs safe

* use Link component in Info block, for target safety

* add eslint rule for unsafe `target` usage

.eslintrc

@@ -24,6 +24,13 @@
     "import"
   ],
 
+  "settings": {
+    "react": {
+      "pragma": "React",
+      "version": "15.0"
+    }
+  },
+
   "rules": {
     "semi": [2, "never"],
     "strict": 0,
@@ -37,6 +44,7 @@
     "comma-dangle": 0,
     "no-console": ["error", { allow: ["warn", "error"] }],
     "react/jsx-no-bind": 1,
+    "react/jsx-no-target-blank": 2,
     "react/display-name": 0,
     "mocha/no-exclusive-tests": "error",
     "import/no-extraneous-dependencies": [2]

package-lock.json

@@ -4612,15 +4612,36 @@
       "dev": true
     },
     "eslint-plugin-react": {
-      "version": "7.7.0",
+      "version": "7.10.0",
-      "resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-7.7.0.tgz",
+      "resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-7.10.0.tgz",
-      "integrity": "sha512-KC7Snr4YsWZD5flu6A5c0AcIZidzW3Exbqp7OT67OaD2AppJtlBr/GuPrW/vaQM/yfZotEvKAdrxrO+v8vwYJA==",
+      "integrity": "sha512-18rzWn4AtbSUxFKKM7aCVcj5LXOhOKdwBino3KKWy4psxfPW0YtIbE8WNRDUdyHFL50BeLb6qFd4vpvNYyp7hw==",
       "dev": true,
       "requires": {
         "doctrine": "2.1.0",
-        "has": "1.0.1",
+        "has": "1.0.3",
         "jsx-ast-utils": "2.0.1",
-        "prop-types": "15.6.1"
+        "prop-types": "15.6.2"
+      },
+      "dependencies": {
+        "has": {
+          "version": "1.0.3",
+          "resolved": "https://registry.npmjs.org/has/-/has-1.0.3.tgz",
+          "integrity": "sha512-f2dvO0VU6Oej7RkWJGrehjbzMAjFp5/VKPp5tTpWIV4JHHZK1/BxbFRtf/siA2SWTe09caDmVtYYzWEIbBS4zw==",
+          "dev": true,
+          "requires": {
+            "function-bind": "1.1.1"
+          }
+        },
+        "prop-types": {
+          "version": "15.6.2",
+          "resolved": "https://registry.npmjs.org/prop-types/-/prop-types-15.6.2.tgz",
+          "integrity": "sha512-3pboPvLiWD7dkI3qf3KbUe6hKFKa52w+AE0VCqECtf+QHAKgOL37tTaNCnuX1nAAQ4ZhyP+kYVKf8rLmJ/feDQ==",
+          "dev": true,
+          "requires": {
+            "loose-envify": "1.3.1",
+            "object-assign": "4.1.1"
+          }
+        }
       }
     },
     "eslint-plugin-standard": {
@@ -17778,6 +17799,18 @@
             }
           }
         },
+        "eslint-plugin-react": {
+          "version": "7.7.0",
+          "resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-7.7.0.tgz",
+          "integrity": "sha512-KC7Snr4YsWZD5flu6A5c0AcIZidzW3Exbqp7OT67OaD2AppJtlBr/GuPrW/vaQM/yfZotEvKAdrxrO+v8vwYJA==",
+          "dev": true,
+          "requires": {
+            "doctrine": "2.1.0",
+            "has": "1.0.1",
+            "jsx-ast-utils": "2.0.1",
+            "prop-types": "15.6.1"
+          }
+        },
         "globals": {
           "version": "11.5.0",
           "resolved": "https://registry.npmjs.org/globals/-/globals-11.5.0.tgz",

package.json

@@ -100,7 +100,7 @@
     "eslint": "^4.1.1",
     "eslint-plugin-import": "^2.13.0",
     "eslint-plugin-mocha": "^4.11.0",
-    "eslint-plugin-react": "~7.7.0",
+    "eslint-plugin-react": "^7.10.0",
     "expect": "^1.20.2",
     "extract-text-webpack-plugin": "^3.0.2",
     "file-loader": "^1.1.11",

src/core/components/info.jsx

@@ -25,22 +25,25 @@ export class InfoBasePath extends React.Component {
 
 class Contact extends React.Component {
   static propTypes = {
-    data: PropTypes.object
+    data: PropTypes.object,
+    getComponent: PropTypes.func.isRequired
   }
 
   render(){
-    let { data } = this.props
+    let { data, getComponent } = this.props
     let name = data.get("name") || "the developer"
     let url = data.get("url")
     let email = data.get("email")
 
+    const Link = getComponent("Link")
+
     return (
       <div>
-        { url && <div><a href={ sanitizeUrl(url) } target="_blank">{ name } - Website</a></div> }
+        { url && <div><Link href={ sanitizeUrl(url) } target="_blank">{ name } - Website</Link></div> }
         { email &&
-          <a href={sanitizeUrl(`mailto:${email}`)}>
+          <Link href={sanitizeUrl(`mailto:${email}`)}>
             { url ? `Send email to ${name}` : `Contact ${name}`}
-          </a>
+          </Link>
         }
       </div>
     )
@@ -49,18 +52,23 @@ class Contact extends React.Component {
 
 class License extends React.Component {
   static propTypes = {
-    license: PropTypes.object
+    license: PropTypes.object,
+    getComponent: PropTypes.func.isRequired
+
   }
 
   render(){
-    let { license } = this.props
+    let { license, getComponent } = this.props
+
+    const Link = getComponent("Link")
+  
     let name = license.get("name") || "License"
     let url = license.get("url")
 
     return (
       <div>
         {
-          url ? <a target="_blank" href={ sanitizeUrl(url) }>{ name }</a>
+          url ? <Link target="_blank" href={ sanitizeUrl(url) }>{ name }</Link>
         : <span>{ name }</span>
         }
       </div>
@@ -70,12 +78,17 @@ class License extends React.Component {
 
 export class InfoUrl extends React.PureComponent {
   static propTypes = {
-    url: PropTypes.string.isRequired
+    url: PropTypes.string.isRequired,
+    getComponent: PropTypes.func.isRequired
   }
 
+  
   render() {
-    const { url } = this.props
+    const { url, getComponent } = this.props
-    return <a target="_blank" href={ sanitizeUrl(url) }><span className="url"> { url } </span></a>
+
+    const Link = getComponent("Link")
+
+    return <Link target="_blank" href={ sanitizeUrl(url) }><span className="url"> { url } </span></Link>
   }
 }
 
@@ -100,6 +113,7 @@ export default class Info extends React.Component {
     const { url:externalDocsUrl, description:externalDocsDescription } = (externalDocs || fromJS({})).toJS()
 
     const Markdown = getComponent("Markdown")
+    const Link = getComponent("Link")
     const VersionStamp = getComponent("VersionStamp")
     const InfoUrl = getComponent("InfoUrl")
     const InfoBasePath = getComponent("InfoBasePath")
@@ -111,7 +125,7 @@ export default class Info extends React.Component {
             { version && <VersionStamp version={version}></VersionStamp> }
           </h2>
           { host || basePath ? <InfoBasePath host={ host } basePath={ basePath } /> : null }
-          { url && <InfoUrl url={url} /> }
+          { url && <InfoUrl getComponent={getComponent} url={url} /> }
         </hgroup>
 
         <div className="description">
@@ -120,14 +134,14 @@ export default class Info extends React.Component {
 
         {
           termsOfService && <div>
-            <a target="_blank" href={ sanitizeUrl(termsOfService) }>Terms of service</a>
+            <Link target="_blank" href={ sanitizeUrl(termsOfService) }>Terms of service</Link>
           </div>
         }
 
-        { contact && contact.size ? <Contact data={ contact } /> : null }
+        {contact && contact.size ? <Contact getComponent={getComponent} data={ contact } /> : null }
-        { license && license.size ? <License license={ license } /> : null }
+        {license && license.size ? <License getComponent={getComponent} license={ license } /> : null }
         { externalDocsUrl ?
-            <a target="_blank" href={sanitizeUrl(externalDocsUrl)}>{externalDocsDescription || externalDocsUrl}</a>
+            <Link target="_blank" href={sanitizeUrl(externalDocsUrl)}>{externalDocsDescription || externalDocsUrl}</Link>
         : null }
 
       </div>

src/core/components/layout-utils.jsx

@@ -196,7 +196,7 @@ export class Select extends React.Component {
 export class Link extends React.Component {
 
   render() {
-    return <a {...this.props} className={xclass(this.props.className, "link")}/>
+    return <a {...this.props} rel="noopener noreferrer" className={xclass(this.props.className, "link")}/>
   }
 
 }

src/core/components/online-validator-badge.jsx

@@ -54,7 +54,7 @@ export default class OnlineValidatorBadge extends React.Component {
         }
 
         return (<span style={{ float: "right"}}>
-                <a target="_blank" href={`${ sanitizedValidatorUrl }/debug?url=${ encodeURIComponent(this.state.url) }`}>
+                <a target="_blank" rel="noopener noreferrer" href={`${ sanitizedValidatorUrl }/debug?url=${ encodeURIComponent(this.state.url) }`}>
                     <ValidatorImage src={`${ sanitizedValidatorUrl }?url=${ encodeURIComponent(this.state.url) }`} alt="Online validator badge"/>
                 </a>
             </span>)

src/core/components/operation-tag.jsx

@@ -46,6 +46,7 @@ export default class OperationTag extends React.Component {
     const Collapse = getComponent("Collapse")
     const Markdown = getComponent("Markdown")
     const DeepLink = getComponent("DeepLink")
+    const Link = getComponent("Link")
 
     let tagDescription = tagObj.getIn(["tagDetails", "description"], null)
     let tagExternalDocsDescription = tagObj.getIn(["tagDetails", "externalDocs", "description"])
@@ -78,11 +79,11 @@ export default class OperationTag extends React.Component {
                     { tagExternalDocsDescription }
                       { tagExternalDocsUrl ? ": " : null }
                       { tagExternalDocsUrl ?
-                        <a
+                        <Link
                             href={sanitizeUrl(tagExternalDocsUrl)}
                             onClick={(e) => e.stopPropagation()}
-                            target={"_blank"}
+                            target="_blank"
-                            >{tagExternalDocsUrl}</a> : null
+                            >{tagExternalDocsUrl}</Link> : null
                           }
                   </small>
                 }

src/core/components/operation.jsx

@@ -99,6 +99,7 @@ export default class Operation extends PureComponent {
     const OperationServers = getComponent( "OperationServers" )
     const OperationExt = getComponent( "OperationExt" )
     const OperationSummary = getComponent( "OperationSummary" )
+    const Link = getComponent( "Link" )
 
     const { showExtensions } = getConfigs()
 
@@ -134,7 +135,7 @@ export default class Operation extends PureComponent {
                     <span className="opblock-external-docs__description">
                       <Markdown source={ externalDocs.description } />
                     </span>
-                    <a target="_blank" className="opblock-external-docs__link" href={ sanitizeUrl(externalDocs.url) }>{ externalDocs.url }</a>
+                    <Link target="_blank" className="opblock-external-docs__link" href={sanitizeUrl(externalDocs.url)}>{externalDocs.url}</Link>
                   </div>
                 </div> : null
               }

src/core/components/providers/markdown.jsx

@@ -4,6 +4,17 @@ import Remarkable from "remarkable"
 import DomPurify from "dompurify"
 import cx from "classnames"
 
+DomPurify.addHook("beforeSanitizeElements", function (current, ) {
+  // Attach safe `rel` values to all elements that contain an `href`,
+  // i.e. all anchors that are links.
+  // We _could_ just look for elements that have a non-self target,
+  // but applying it more broadly shouldn't hurt anything, and is safer.
+  if (current.href) {
+    current.setAttribute("rel", "noopener noreferrer")
+  }
+  return current
+})
+
 // eslint-disable-next-line no-useless-escape
 const isPlainText = (str) => /^[A-Z\s0-9!?\.]+$/gi.test(str)
 
@@ -15,13 +26,16 @@ function Markdown({ source, className = "" }) {
         {source}
       </div>
     }
-    const html = new Remarkable({
+
+    const md = new Remarkable({
         html: true,
         typographer: true,
         breaks: true,
         linkify: true,
         linkTarget: "_blank"
-    }).render(source)
+    })
+    
+    const html = md.render(source)
     const sanitized = sanitizer(html)
 
     if ( !source || !html || !sanitized ) {

src/core/plugins/oas3/wrap-components/markdown.js

@@ -7,6 +7,8 @@ import { sanitizer } from "core/components/providers/markdown"
 
 const parser = new Remarkable("commonmark")
 
+parser.set({ linkTarget: "_blank" })
+
 export const Markdown = ({ source, className = "" }) => {
   if ( source ) {
     const html = parser.render(source)

test/components/markdown.js

@@ -52,7 +52,7 @@ describe("Markdown component", function() {
         it("allows links", function() {
             const str = `[Link](https://example.com/)`
             const el = render(<Markdown source={str} />)
-            expect(el.html()).toEqual(`<div class="markdown"><p><a target="_blank" href="https://example.com/">Link</a></p>\n</div>`)
+            expect(el.html()).toEqual(`<div class="markdown"><p><a rel="noopener noreferrer" target="_blank" href="https://example.com/">Link</a></p>\n</div>`)
         })
     })
 

test/xss/anchor-target-rel/info.js

@@ -0,0 +1,108 @@
+/* eslint-env mocha */
+import React from "react"
+import expect from "expect"
+import { render } from "enzyme"
+import { fromJS } from "immutable"
+import Info, { InfoUrl } from "components/info"
+import { Link } from "components/layout-utils"
+import Markdown from "components/providers/markdown"
+
+describe("<Info/> Anchor Target Safety", function(){
+	const dummyComponent = () => null
+	const components = {
+		Markdown,
+		InfoUrl,
+		Link
+	}
+	const baseProps = {
+		getComponent: c => components[c] || dummyComponent,
+		host: "example.test",
+		basePath: "/api",
+		info: fromJS({
+			title: "Hello World"
+		})
+	}
+
+	it("renders externalDocs links with safe `rel` attributes", function () {
+		const props = {
+			...baseProps,
+			externalDocs: fromJS({
+				url: "http://google.com/"
+			})
+		}
+		let wrapper = render(<Info {...props} />)
+		const anchor = wrapper.find("a")
+
+		expect(anchor.html()).toEqual("http://google.com/")
+		expect(anchor.attr("target")).toEqual("_blank")
+		expect(anchor.attr("rel") || "").toInclude("noopener")
+		expect(anchor.attr("rel") || "").toInclude("noreferrer")
+	})
+
+	it("renders Contact links with safe `rel` attributes", function () {
+		const props = {
+			...baseProps,
+			info: fromJS({
+				contact: {
+					url: "http://google.com/",
+					name: "My Site"
+				}
+			})
+		}
+		let wrapper = render(<Info {...props} />)
+		const anchor = wrapper.find("a")
+
+		expect(anchor.attr("href")).toEqual("http://google.com/")
+		expect(anchor.attr("target")).toEqual("_blank")
+		expect(anchor.attr("rel") || "").toInclude("noopener")
+		expect(anchor.attr("rel") || "").toInclude("noreferrer")
+	})
+
+	it("renders License links with safe `rel` attributes", function () {
+		const props = {
+			...baseProps,
+			info: fromJS({
+				license: {
+					url: "http://mit.edu/"
+				}
+			})
+		}
+		let wrapper = render(<Info {...props} />)
+		const anchor = wrapper.find("a")
+
+		expect(anchor.attr("href")).toEqual("http://mit.edu/")
+		expect(anchor.attr("target")).toEqual("_blank")
+		expect(anchor.attr("rel") || "").toInclude("noopener")
+		expect(anchor.attr("rel") || "").toInclude("noreferrer")
+	})
+
+	it("renders termsOfService links with safe `rel` attributes", function () {
+		const props = {
+			...baseProps,
+			info: fromJS({
+				termsOfService: "http://smartbear.com/"
+			})
+		}
+		let wrapper = render(<Info {...props} />)
+		const anchor = wrapper.find("a")
+
+		expect(anchor.attr("href")).toEqual("http://smartbear.com/")
+		expect(anchor.attr("target")).toEqual("_blank")
+		expect(anchor.attr("rel") || "").toInclude("noopener")
+		expect(anchor.attr("rel") || "").toInclude("noreferrer")
+	})
+
+	it("renders definition URL links with safe `rel` attributes", function () {
+		const props = {
+			...baseProps,
+			url: "http://petstore.swagger.io/v2/petstore.json"
+		}
+		let wrapper = render(<Info {...props} />)
+		const anchor = wrapper.find("a")
+
+		expect(anchor.attr("href")).toEqual("http://petstore.swagger.io/v2/petstore.json")
+		expect(anchor.attr("target")).toEqual("_blank")
+		expect(anchor.attr("rel") || "").toInclude("noopener")
+		expect(anchor.attr("rel") || "").toInclude("noreferrer")
+	})
+})

test/xss/anchor-target-rel/link.js

@@ -0,0 +1,44 @@
+/* eslint-env mocha */
+import React from "react"
+import expect from "expect"
+import { render } from "enzyme"
+import { fromJS } from "immutable"
+import { Link } from "components/layout-utils"
+
+describe("<Link/> Anchor Target Safety", function () {
+  const dummyComponent = () => null
+  const components = {
+    Link
+  }
+  const baseProps = {
+    getComponent: c => components[c] || dummyComponent
+  }
+
+  it("renders regular links with `noreferrer` and `noopener`", function () {
+    const props = {
+      ...baseProps,
+      href: "http://google.com/"
+    }
+    let wrapper = render(<Link {...props} />)
+    const anchor = wrapper.find("a")
+
+    expect(anchor.attr("href")).toEqual("http://google.com/")
+    expect(anchor.attr("rel") || "").toInclude("noopener")
+    expect(anchor.attr("rel") || "").toInclude("noreferrer")
+  })
+
+  it("enforces `noreferrer` and `noopener` on target=_blank links", function () {
+    const props = {
+      ...baseProps,
+      href: "http://google.com/",
+      target: "_blank"
+    }
+    let wrapper = render(<Link {...props} />)
+    const anchor = wrapper.find("a")
+
+    expect(anchor.attr("href")).toEqual("http://google.com/")
+    expect(anchor.attr("target")).toEqual("_blank")
+    expect(anchor.attr("rel") || "").toInclude("noopener")
+    expect(anchor.attr("rel") || "").toInclude("noreferrer")
+  })
+})

test/xss/anchor-target-rel/markdown.js

@@ -0,0 +1,66 @@
+/* eslint-env mocha */
+import React from "react"
+import expect from "expect"
+import { render } from "enzyme"
+import Markdown from "components/providers/markdown"
+import { Markdown as OAS3Markdown } from "corePlugins/oas3/wrap-components/markdown.js"
+
+describe("Markdown Link Anchor Safety", function () {
+  describe("Swagger 2.0", function () {
+    it("sanitizes Markdown links", function () {
+      const str = `Hello, [here](http://google.com/) is my link`
+      const wrapper = render(<Markdown source={str} />)
+
+      const anchor = wrapper.find("a")
+
+      expect(anchor.attr("href")).toEqual("http://google.com/")
+      expect(anchor.attr("target")).toEqual("_blank")
+      expect(anchor.attr("rel") || "").toInclude("noopener")
+      expect(anchor.attr("rel") || "").toInclude("noreferrer")
+    })
+
+    it("sanitizes raw HTML links", function () {
+      const str = `Hello, <a href="http://google.com/">here</a> is my link`
+      const wrapper = render(<Markdown source={str} />)
+
+      const anchor = wrapper.find("a")
+
+      expect(anchor.attr("href")).toEqual("http://google.com/")
+      expect(anchor.attr("rel") || "").toInclude("noopener")
+      expect(anchor.attr("rel") || "").toInclude("noreferrer")
+    })
+  })
+
+  describe("OAS 3", function () {
+    it("sanitizes Markdown links", function () {
+      const str = `Hello, [here](http://google.com/) is my link`
+      const wrapper = render(<OAS3Markdown source={str} />)
+
+      const anchor = wrapper.find("a")
+
+      expect(anchor.attr("href")).toEqual("http://google.com/")
+      expect(anchor.attr("target")).toEqual("_blank")
+      expect(anchor.attr("rel") || "").toInclude("noopener")
+      expect(anchor.attr("rel") || "").toInclude("noreferrer")
+    })
+
+    it("sanitizes raw HTML links", function () {
+      const str = `Hello, <a href="http://google.com/">here</a> is my link`
+      const wrapper = render(<OAS3Markdown source={str} />)
+
+      const anchor = wrapper.find("a")
+
+      expect(anchor.attr("href")).toEqual("http://google.com/")
+      expect(anchor.attr("rel") || "").toInclude("noopener")
+      expect(anchor.attr("rel") || "").toInclude("noreferrer")
+    })
+  })
+})
+
+function withMarkdownWrapper(str, { isOAS3 = false } = {}) {
+  if(isOAS3) {
+    return `<div class="renderedMarkdown"><p>${str}</p></div>`
+  }
+
+  return `<div class="markdown"><p>${str}</p>\n</div>`
+}

test/xss/anchor-target-rel/online-validator-badge.js

@@ -0,0 +1,32 @@
+/* eslint-env mocha */
+import React from "react"
+import expect, { createSpy } from "expect"
+import { mount } from "enzyme"
+import { fromJS, Map } from "immutable"
+import OnlineValidatorBadge from "components/online-validator-badge"
+
+describe("<OnlineValidatorBadge/> Anchor Target Safety", function () {
+  it("should render a validator link with safe `rel` attributes", function () {
+    // When
+    const props = {
+      getConfigs: () => ({}),
+      getComponent: () => null,
+      specSelectors: {
+        url: () => "swagger.json"
+      }
+    }
+    const wrapper = mount(
+     <OnlineValidatorBadge {...props} />
+    )
+
+    const anchor = wrapper.find("a")
+
+    // Then
+    expect(anchor.props().href).toEqual(
+      "https://online.swagger.io/validator/debug?url=swagger.json"
+    )
+    expect(anchor.props().target).toEqual("_blank")
+    expect(anchor.props().rel || "").toInclude("noopener")
+    expect(anchor.props().rel || "").toInclude("noreferrer")
+  })
+})