Skip to content

Commit

Permalink
fix(security): disable reading config params from URL search params (#…
Browse files Browse the repository at this point in the history
…7697)

Reading configuration parameters from URL search params 
is by default no longer enabled. To re-enable it, set queryConfigEnabled
configuration parameter to true.

Functionally, this is a breaking change, but given we're just providing
a security vulnerability patch we're considering this a PATCH version bump
only.

Refs #4872
Refs GHSA-qrmm-w75w-3wpx
  • Loading branch information
char0n authored Dec 9, 2021
1 parent df7749b commit 01a3e55
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 9 deletions.
17 changes: 12 additions & 5 deletions docker/configurator/translator.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ const defaultBaseConfig = {
type: "string",
base: true
}
},
queryConfigEnabled: {
value: "false",
schema: {
type: "boolean",
base: true,
}
}
}

Expand All @@ -51,14 +58,14 @@ function objectToKeyValueString(env, { injectBaseConfig = false, schema = config
const keys = Object.keys(env)

// Compute an intermediate representation that holds candidate values and schemas.
//
//
// This is useful for deduping between multiple env keys that set the same
// config variable.

keys.forEach(key => {
const varSchema = schema[key]
const value = env[key]

if(!varSchema) return

if(varSchema.onFound) {
Expand Down Expand Up @@ -88,8 +95,8 @@ function objectToKeyValueString(env, { injectBaseConfig = false, schema = config

Object.keys(valueStorage).forEach(key => {
const value = valueStorage[key]
const escapedName = /[^a-zA-Z0-9]/.test(key) ? `"${key}"` : key

const escapedName = /[^a-zA-Z0-9]/.test(key) ? `"${key}"` : key

if (value.schema.type === "string") {
result += `${escapedName}: "${value.value}",\n`
Expand All @@ -101,4 +108,4 @@ function objectToKeyValueString(env, { injectBaseConfig = false, schema = config
return result.trim()
}

module.exports = objectToKeyValueString
module.exports = objectToKeyValueString
4 changes: 4 additions & 0 deletions docker/configurator/variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ const standardVariables = {
type: "string",
name: "urls.primaryName"
},
QUERY_CONFIG_ENABLED: {
type: "boolean",
name: "queryConfigEnabled"
},
LAYOUT: {
type: "string",
name: "layout"
Expand Down
1 change: 1 addition & 0 deletions docs/usage/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Parameter name | Docker variable | Description
<a name="url"></a>`url` | `URL` | `String`. The URL pointing to API definition (normally `swagger.json` or `swagger.yaml`). Will be ignored if `urls` or `spec` is used.
<a name="urls"></a>`urls` | `URLS` | `Array`. An array of API definition objects (`[{url: "<url1>", name: "<name1>"},{url: "<url2>", name: "<name2>"}]`) used by Topbar plugin. When used and Topbar plugin is enabled, the `url` parameter will not be parsed. Names and URLs must be unique among all items in this array, since they're used as identifiers.
<a name="urls.primaryName"></a>`urls.primaryName` | `URLS_PRIMARY_NAME` | `String`. When using `urls`, you can use this subparameter. If the value matches the name of a spec provided in `urls`, that spec will be displayed when Swagger UI loads, instead of defaulting to the first spec in `urls`.
<a name="queryConfigEnabled"></a>`queryConfigEnabled` | `QUERY_CONFIG_ENABLED` | `Boolean=false`. Enables overriding configuration parameters via URL search params.

##### Plugin system

Expand Down
5 changes: 4 additions & 1 deletion flavors/swagger-ui-react/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default class SwaggerUI extends React.Component {
onComplete: this.onComplete,
docExpansion: this.props.docExpansion,
supportedSubmitMethods: this.props.supportedSubmitMethods,
queryConfigEnabled: this.props.queryConfigEnabled,
defaultModelExpandDepth: this.props.defaultModelExpandDepth,
displayOperationId: this.props.displayOperationId,
tryItOutEnabled: this.props.tryItOutEnabled,
Expand All @@ -29,7 +30,7 @@ export default class SwaggerUI extends React.Component {
showMutatedRequest: typeof this.props.showMutatedRequest === "boolean" ? this.props.showMutatedRequest : true,
deepLinking: typeof this.props.deepLinking === "boolean" ? this.props.deepLinking : false,
showExtensions: this.props.showExtensions,
filter: ["boolean", "string"].includes(typeof this.props.filter) ? this.props.filter : false,
filter: ["boolean", "string"].includes(typeof this.props.filter) ? this.props.filter : false,
})

this.system = ui
Expand Down Expand Up @@ -99,6 +100,7 @@ SwaggerUI.propTypes = {
supportedSubmitMethods: PropTypes.arrayOf(
PropTypes.oneOf(["get", "put", "post", "delete", "options", "head", "patch", "trace"])
),
queryConfigEnabled: PropTypes.bool,
plugins: PropTypes.arrayOf(PropTypes.object),
displayOperationId: PropTypes.bool,
showMutatedRequest: PropTypes.bool,
Expand All @@ -119,6 +121,7 @@ SwaggerUI.propTypes = {
SwaggerUI.defaultProps = {
layout: "BaseLayout",
supportedSubmitMethods: ["get", "put", "post", "delete", "options", "head", "patch", "trace"],
queryConfigEnabled: false,
docExpansion: "list",
defaultModelsExpandDepth: 1,
presets: [],
Expand Down
3 changes: 2 additions & 1 deletion src/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export default function SwaggerUI(opts) {
"patch",
"trace"
],
queryConfigEnabled: false,

// Initial set of plugins ( TODO rename this, or refactor - we don't need presets _and_ plugins. Its just there for performance.
// Instead, we can compile the first plugin ( it can be a collection of plugins ), then batch the rest.
Expand Down Expand Up @@ -108,7 +109,7 @@ export default function SwaggerUI(opts) {
}
}

let queryConfig = parseSearch()
let queryConfig = opts.queryConfigEnabled ? parseSearch() : {}

const domNode = opts.domNode
delete opts.domNode
Expand Down
3 changes: 2 additions & 1 deletion test/e2e-cypress/static/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@
} else {
window.completeCount = 1
}
}
},
queryConfigEnabled: true,
})

window.ui = ui
Expand Down
3 changes: 2 additions & 1 deletion test/e2e-cypress/static/pages/5138/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@
} else {
window.completeCount = 1
}
}
},
queryConfigEnabled: true,
})

window.ui = ui
Expand Down
3 changes: 3 additions & 0 deletions test/unit/docker/translator.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe("docker: env translator", function() {
SwaggerUIBundle.plugins.DownloadUrl
],
layout: "StandaloneLayout",
queryConfigEnabled: false,
`))
})

Expand Down Expand Up @@ -214,6 +215,7 @@ describe("docker: env translator", function() {
SwaggerUIBundle.plugins.DownloadUrl
],
layout: "StandaloneLayout",
queryConfigEnabled: false,
url: "/swagger.json",
urls: ["/one", "/two"],`

Expand Down Expand Up @@ -313,6 +315,7 @@ describe("docker: env translator", function() {
plugins: [
SwaggerUIBundle.plugins.DownloadUrl
],
queryConfigEnabled: false,
configUrl: "/wow",
"dom_id": "#swagger_ui",
spec: { swagger: "2.0" },
Expand Down

0 comments on commit 01a3e55

Please sign in to comment.