fix: mitigate "sequential @import chaining" vulnerability (#5616)
* `test/e2e-cypress/tests/features/xss/` -> `test/e2e-cypress/tests/security` * add tests * filter <style> tags out of Markdown fields * initialize OAuth inputs without applying `value` attribute
This commit is contained in:
@@ -96,6 +96,7 @@ export default class Oauth2 extends React.Component {
|
|||||||
const AuthError = getComponent("authError")
|
const AuthError = getComponent("authError")
|
||||||
const JumpToPath = getComponent("JumpToPath", true)
|
const JumpToPath = getComponent("JumpToPath", true)
|
||||||
const Markdown = getComponent( "Markdown" )
|
const Markdown = getComponent( "Markdown" )
|
||||||
|
const InitializedInput = getComponent("InitializedInput")
|
||||||
|
|
||||||
const { isOAS3 } = specSelectors
|
const { isOAS3 } = specSelectors
|
||||||
|
|
||||||
@@ -170,10 +171,10 @@ export default class Oauth2 extends React.Component {
|
|||||||
{
|
{
|
||||||
isAuthorized ? <code> ****** </code>
|
isAuthorized ? <code> ****** </code>
|
||||||
: <Col tablet={10} desktop={10}>
|
: <Col tablet={10} desktop={10}>
|
||||||
<input id="client_id"
|
<InitializedInput id="client_id"
|
||||||
type="text"
|
type="text"
|
||||||
required={ flow === PASSWORD }
|
required={ flow === PASSWORD }
|
||||||
value={ this.state.clientId }
|
initialValue={ this.state.clientId }
|
||||||
data-name="clientId"
|
data-name="clientId"
|
||||||
onChange={ this.onInputChange }/>
|
onChange={ this.onInputChange }/>
|
||||||
</Col>
|
</Col>
|
||||||
@@ -187,8 +188,8 @@ export default class Oauth2 extends React.Component {
|
|||||||
{
|
{
|
||||||
isAuthorized ? <code> ****** </code>
|
isAuthorized ? <code> ****** </code>
|
||||||
: <Col tablet={10} desktop={10}>
|
: <Col tablet={10} desktop={10}>
|
||||||
<input id="client_secret"
|
<InitializedInput id="client_secret"
|
||||||
value={ this.state.clientSecret }
|
initialValue={ this.state.clientSecret }
|
||||||
type="text"
|
type="text"
|
||||||
data-name="clientSecret"
|
data-name="clientSecret"
|
||||||
onChange={ this.onInputChange }/>
|
onChange={ this.onInputChange }/>
|
||||||
|
|||||||
36
src/core/components/initialized-input.jsx
Normal file
36
src/core/components/initialized-input.jsx
Normal file
@@ -0,0 +1,36 @@
|
|||||||
|
// This component provides an interface that feels like an uncontrolled input
|
||||||
|
// to consumers, while providing a `defaultValue` interface that initializes
|
||||||
|
// the input's value using JavaScript value property APIs instead of React's
|
||||||
|
// vanilla[0] implementation that uses HTML value attributes.
|
||||||
|
//
|
||||||
|
// This is useful in situations where we don't want to surface an input's value
|
||||||
|
// into the HTML/CSS-exposed side of the DOM, for example to avoid sequential
|
||||||
|
// input chaining attacks[1].
|
||||||
|
//
|
||||||
|
// [0]: https://github.com/facebook/react/blob/baff5cc2f69d30589a5dc65b089e47765437294b/fixtures/dom/src/components/fixtures/text-inputs/README.md
|
||||||
|
// [1]: https://github.com/d0nutptr/sic
|
||||||
|
|
||||||
|
import React from "react"
|
||||||
|
import PropTypes from "prop-types"
|
||||||
|
|
||||||
|
export default class InitializedInput extends React.Component {
|
||||||
|
componentDidMount() {
|
||||||
|
// Set the element's `value` property (*not* the `value` attribute)
|
||||||
|
// once, on mount, if an `initialValue` is provided.
|
||||||
|
if(this.props.initialValue) {
|
||||||
|
this.inputRef.value = this.props.initialValue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
render() {
|
||||||
|
// Filter out `value` and `defaultValue`, since we have our own
|
||||||
|
// `initialValue` interface that we provide.
|
||||||
|
// eslint-disable-next-line no-unused-vars, react/prop-types
|
||||||
|
const { value, defaultValue, ...otherProps } = this.props
|
||||||
|
return <input {...otherProps} ref={c => this.inputRef = c} />
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
InitializedInput.propTypes = {
|
||||||
|
initialValue: PropTypes.string
|
||||||
|
}
|
||||||
@@ -51,6 +51,7 @@ export default Markdown
|
|||||||
|
|
||||||
export function sanitizer(str) {
|
export function sanitizer(str) {
|
||||||
return DomPurify.sanitize(str, {
|
return DomPurify.sanitize(str, {
|
||||||
ADD_ATTR: ["target"]
|
ADD_ATTR: ["target"],
|
||||||
|
FORBID_TAGS: ["style"],
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -53,6 +53,7 @@ import Headers from "core/components/headers"
|
|||||||
import Errors from "core/components/errors"
|
import Errors from "core/components/errors"
|
||||||
import ContentType from "core/components/content-type"
|
import ContentType from "core/components/content-type"
|
||||||
import Overview from "core/components/overview"
|
import Overview from "core/components/overview"
|
||||||
|
import InitializedInput from "core/components/initialized-input"
|
||||||
import Info, {
|
import Info, {
|
||||||
InfoUrl,
|
InfoUrl,
|
||||||
InfoBasePath
|
InfoBasePath
|
||||||
@@ -105,6 +106,7 @@ export default function() {
|
|||||||
basicAuth: BasicAuth,
|
basicAuth: BasicAuth,
|
||||||
clear: Clear,
|
clear: Clear,
|
||||||
liveResponse: LiveResponse,
|
liveResponse: LiveResponse,
|
||||||
|
InitializedInput,
|
||||||
info: Info,
|
info: Info,
|
||||||
InfoContainer,
|
InfoContainer,
|
||||||
JumpToPath,
|
JumpToPath,
|
||||||
|
|||||||
@@ -13,6 +13,8 @@ info:
|
|||||||
url: https://www.apache.org/licenses/LICENSE-2.0.html
|
url: https://www.apache.org/licenses/LICENSE-2.0.html
|
||||||
servers:
|
servers:
|
||||||
- url: http://petstore.swagger.io/api
|
- url: http://petstore.swagger.io/api
|
||||||
|
security:
|
||||||
|
- Petstore: []
|
||||||
paths:
|
paths:
|
||||||
/pets:
|
/pets:
|
||||||
get:
|
get:
|
||||||
@@ -153,3 +155,12 @@ components:
|
|||||||
format: int32
|
format: int32
|
||||||
message:
|
message:
|
||||||
type: string
|
type: string
|
||||||
|
securitySchemes:
|
||||||
|
Petstore:
|
||||||
|
type: oauth2
|
||||||
|
flows:
|
||||||
|
implicit:
|
||||||
|
authorizationUrl: https://example.com/api/oauth/dialog
|
||||||
|
scopes:
|
||||||
|
write:pets: modify pets in your account
|
||||||
|
read:pets: read your pets
|
||||||
|
|||||||
@@ -0,0 +1,7 @@
|
|||||||
|
* {
|
||||||
|
color: red !important; /* for humans */
|
||||||
|
}
|
||||||
|
|
||||||
|
h4 {
|
||||||
|
display: none; /* for machines, used to trace whether this sheet is applied */
|
||||||
|
}
|
||||||
@@ -0,0 +1,10 @@
|
|||||||
|
openapi: "3.0.0"
|
||||||
|
|
||||||
|
info:
|
||||||
|
title: Sequential Import Chaining
|
||||||
|
description: >
|
||||||
|
<h4>This h4 would be hidden by the injected CSS</h4>
|
||||||
|
|
||||||
|
This document tests the ability of a `<style>` tag in a Markdown field to pull in a remote stylesheet using an `@import` directive.
|
||||||
|
|
||||||
|
<style>@import url(/documents/security/sequential-import-chaining/injection.css);</style>
|
||||||
@@ -0,0 +1,10 @@
|
|||||||
|
swagger: "2.0"
|
||||||
|
|
||||||
|
info:
|
||||||
|
title: Sequential Import Chaining
|
||||||
|
description: >
|
||||||
|
<h4>This h4 would be hidden by the injected CSS</h4>
|
||||||
|
|
||||||
|
This document tests the ability of a `<style>` tag in a Markdown field to pull in a remote stylesheet using an `@import` directive.
|
||||||
|
|
||||||
|
<style>@import url(/documents/security/sequential-import-chaining/injection.css);</style>
|
||||||
@@ -1,6 +1,6 @@
|
|||||||
describe("XSS: OAuth2 authorizationUrl sanitization", () => {
|
describe("XSS: OAuth2 authorizationUrl sanitization", () => {
|
||||||
it("should filter out a javascript URL", () => {
|
it("should filter out a javascript URL", () => {
|
||||||
cy.visit("/?url=/documents/xss/oauth2.yaml")
|
cy.visit("/?url=/documents/security/xss-oauth2.yaml")
|
||||||
.window()
|
.window()
|
||||||
.then(win => {
|
.then(win => {
|
||||||
let args = null
|
let args = null
|
||||||
@@ -0,0 +1,58 @@
|
|||||||
|
describe("Security: CSS Sequential Import Chaining", () => {
|
||||||
|
describe("in OpenAPI 3.0", () => {
|
||||||
|
describe("CSS Injection via Markdown", () => {
|
||||||
|
it("should filter <style> tags out of Markdown fields", () => {
|
||||||
|
cy.visit("/?url=/documents/security/sequential-import-chaining/openapi.yaml")
|
||||||
|
.get("div.information-container")
|
||||||
|
.should("exist")
|
||||||
|
.and("not.have.descendants", "style")
|
||||||
|
})
|
||||||
|
it("should not apply `@import`ed CSS stylesheets", () => {
|
||||||
|
cy.visit("/?url=/documents/security/sequential-import-chaining/openapi.yaml")
|
||||||
|
.wait(500) // HACK: wait for CSS import to settle
|
||||||
|
.get("div.info h4")
|
||||||
|
.should("have.length", 1)
|
||||||
|
.and("not.be.hidden")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
describe("Value Exfiltration via CSS", () => {
|
||||||
|
it("should not allow OAuth credentials to be visible via HTML `value` attribute", () => {
|
||||||
|
cy.visit("/?url=/documents/petstore-expanded.openapi.yaml")
|
||||||
|
.get(".scheme-container > .schemes > .auth-wrapper > .btn > span")
|
||||||
|
.click()
|
||||||
|
.get("div > div > .wrapper > .block-tablet > #client_id")
|
||||||
|
.clear()
|
||||||
|
.type("abc")
|
||||||
|
.should("not.have.attr", "value", "abc")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
describe("in Swagger 2.0", () => {
|
||||||
|
describe("CSS Injection via Markdown", () => {
|
||||||
|
it("should filter <style> tags out of Markdown fields", () => {
|
||||||
|
cy.visit("/?url=/documents/security/sequential-import-chaining/swagger.yaml")
|
||||||
|
.get("div.information-container")
|
||||||
|
.should("exist")
|
||||||
|
.and("not.have.descendants", "style")
|
||||||
|
})
|
||||||
|
it("should not apply `@import`ed CSS stylesheets", () => {
|
||||||
|
cy.visit("/?url=/documents/security/sequential-import-chaining/swagger.yaml")
|
||||||
|
.wait(500) // HACK: wait for CSS import to settle
|
||||||
|
.get("div.info h4")
|
||||||
|
.should("have.length", 1)
|
||||||
|
.and("not.be.hidden")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
describe("Value Exfiltration via CSS", () => {
|
||||||
|
it("should not allow OAuth credentials to be visible via HTML `value` attribute", () => {
|
||||||
|
cy.visit("/?url=/documents/petstore.swagger.yaml")
|
||||||
|
.get(".scheme-container > .schemes > .auth-wrapper > .btn > span")
|
||||||
|
.click()
|
||||||
|
.get("div > div > .wrapper > .block-tablet > #client_id")
|
||||||
|
.clear()
|
||||||
|
.type("abc")
|
||||||
|
.should("not.have.attr", "value", "abc")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
Reference in New Issue
Block a user