Skip to content

Commit

Permalink
Merge pull request from GHSA-4852-vrh7-28rf
Browse files Browse the repository at this point in the history
* fix: security vulnerability allowing XSS reflection with user input

* fix: update docs, replace santize-html and sanitizeUrl with xss module
  • Loading branch information
acao committed Jun 7, 2020
1 parent 40c35fc commit bf1883d
Show file tree
Hide file tree
Showing 14 changed files with 447 additions and 92 deletions.
118 changes: 75 additions & 43 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
> **SECURITY WARNING:** This `graphql-playground-html` and [all of it's middleware dependents](#impacted-packages) in this repository **had a severe XSS Reflection attack vulnerability to unsanitized user input** until version `graphql-playground-html@1.6.20`. Impacted are any and all **user-defined** input to `renderPlaygroundPage()`, `koaPlayground()`,`expressPlayground()`, `koaPlayground()`, or `lambdaPlayground()`. If you used static values, such as `graphql-playground-electron` does in [it's webpack config](https://github.com/prisma-labs/graphql-playground/blob/master/packages/graphql-playground-electron/webpack.config.build.js#L16), you were not vulnerable to the attack. [More Details](./SECURITY.md)
<p align="center"><img src="https://imgur.com/5fzMbyV.png" width="269"></p>

[![npm version](https://badge.fury.io/js/graphql-playground-react.svg)](https://badge.fury.io/js/graphql-playground-react)
[![npm version](https://badge.fury.io/js/graphql-playground-react.svg)](https://badge.fury.io/js/graphql-playground-react)
[![prisma-labs](https://circleci.com/gh/prisma-labs/graphql-playground.svg?style=shield)](https://circleci.com/gh/prisma-labs/graphql-playground)

**Future of this repository**: see [the announcement issue](https://github.com/prisma-labs/graphql-playground/issues/1143) for details.
Expand All @@ -25,6 +27,32 @@ $ brew cask install graphql-playground
- ⚙ GraphQL Config support with multiple Projects & Endpoints
- 🚥 Apollo Tracing support

## Security Details

**NOTE: only _unsanitized user input_ to the functions in these packages is vulnerable** to the recently reported XSS Reflection attack.

### Impact

The only reason this vulnerability exists is because we are using template strings in `renderPlaygroundPage()` with potentially user defined variables. This allows an attacker to inject html and javascript into a page on execution.

Common examples may be user-defined path parameters, query string, unsanitized UI provided values in database, etc that are used to build template strings or passed directly to a `renderPlaygroundPage()` or the matching middleware function equivalent.

### Impacted Packages

**All versions of these packages are impacted until the ones specified below**, which are now safe for user defined input:

- `graphql-playground-html`: **☔ safe** @ `1.6.20`
- `graphql-playground-express` **☔ safe** @ `1.7.15`
- `graphql-playground-koa` **☔ safe** @ `1.6.14`
- `graphql-playground-hapi` **☔ safe** @ `1.6.12`
- `graphql-playground-lambda` **☔ safe** @ `1.7.16`
- `graphql-playground-electron` has always been **☔ safe** from XSS attacks! This is because configuration is statically defined [it's webpack config](https://github.com/prisma-labs/graphql-playground/blob/master/packages/graphql-playground-electron/webpack.config.build.js#L16)
- `graphql-playground-react` is safe because it does not use `renderPlaygroundPage()` anywhere, and thus is not susceptible to template string XSS reflection attacks.

### More Information

See the [security docs](./SECURITY.md) for more details on how your implementation might be impacted by this vulnerability. It contains safe examples, unsafe examples, workarounds, and more details.

## FAQ

### How is this different from [GraphiQL](https://github.com/graphql/graphiql)?
Expand Down Expand Up @@ -122,12 +150,12 @@ interface ISettings {

```ts
interface Tab {
endpoint: string
query: string
name?: string
variables?: string
responses?: string[]
headers?: { [key: string]: string }
endpoint: string
query: string
name?: string
variables?: string
responses?: string[]
headers?: { [key: string]: string }
}
```

Expand Down Expand Up @@ -169,8 +197,8 @@ Including Fonts (`1.`)

```html
<link
href="https://fonts.googleapis.com/css?family=Open+Sans:300,400,600,700|Source+Code+Pro:400,700"
rel="stylesheet"
href="https://fonts.googleapis.com/css?family=Open+Sans:300,400,600,700|Source+Code+Pro:400,700"
rel="stylesheet"
/>
```

Expand All @@ -183,10 +211,10 @@ import { Provider } from 'react-redux'
import { Playground, store } from 'graphql-playground-react'

ReactDOM.render(
<Provider store={store}>
<Playground endpoint="https://api.graph.cool/simple/v1/swapi" />
</Provider>,
document.body,
<Provider store={store}>
<Playground endpoint='https://api.graph.cool/simple/v1/swapi' />
</Provider>,
document.body,
)
```

Expand Down Expand Up @@ -232,18 +260,18 @@ import lambdaPlayground from 'graphql-playground-middleware-lambda'
// const lambdaPlayground = require('graphql-playground-middleware-lambda').default

exports.graphqlHandler = function graphqlHandler(event, context, callback) {
function callbackFilter(error, output) {
// eslint-disable-next-line no-param-reassign
output.headers['Access-Control-Allow-Origin'] = '*'
callback(error, output)
}

const handler = graphqlLambda({ schema: myGraphQLSchema })
return handler(event, context, callbackFilter)
function callbackFilter(error, output) {
// eslint-disable-next-line no-param-reassign
output.headers['Access-Control-Allow-Origin'] = '*'
callback(error, output)
}

const handler = graphqlLambda({ schema: myGraphQLSchema })
return handler(event, context, callbackFilter)
}

exports.playgroundHandler = lambdaPlayground({
endpoint: '/dev/graphql',
endpoint: '/dev/graphql',
})
```

Expand All @@ -267,6 +295,10 @@ functions:
cors: true
```

#### Security Issue

There is an [XSS Reflection Vulnerability](./SECURITY.md) when using these middlewares with unsanitized user input before

## Development

```sh
Expand All @@ -285,27 +317,27 @@ These are the available options:

```ts
export interface EditorColours {
property: string
comment: string
punctuation: string
keyword: string
def: string
qualifier: string
attribute: string
number: string
string: string
builtin: string
string2: string
variable: string
meta: string
atom: string
ws: string
selection: string
cursorColor: string
editorBackground: string
resultBackground: string
leftDrawerBackground: string
rightDrawerBackground: string
property: string
comment: string
punctuation: string
keyword: string
def: string
qualifier: string
attribute: string
number: string
string: string
builtin: string
string2: string
variable: string
meta: string
atom: string
ws: string
selection: string
cursorColor: string
editorBackground: string
resultBackground: string
leftDrawerBackground: string
rightDrawerBackground: string
}
```

Expand Down
132 changes: 132 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Known Vulnerabilities

## XSS Reflection Vulnerability

the origin of the vulnerability is in `renderPlaygroundPage`, found in `graphql-playground-html`

### Impact

When using

- `renderPlaygroundPage()`,
- `koaPlayground()`
- `expressPlayground()`
- `koaPlayground()`
- `lambdaPlayground()`
- any downstream dependents that use these functions

without sanitization of user input, your application is vulnerable to an XSS Reflecton Attack. This is a serious vulnerability that could allow for exfiltration of data or user credentials, or to disrupt systems.

### Impacted Packages

**All versions of these packages are impacted until those specified below**, which are now safe for user defined input:

- `graphql-playground-html`: **☔ safe** @ `1.6.20`
- `graphql-playground-express` **☔ safe** @ `1.7.15`
- `graphql-playground-koa` **☔ safe** @ `1.6.14`
- `graphql-playground-hapi` **☔ safe** @ `1.6.12`
- `graphql-playground-lambda` **☔ safe** @ `1.7.16`

### Static input was always safe

These examples are safe for _all versions_ **because input is static**

with `express` and `renderPlaygroundPage`:

```js
app.get('/playground', (req) => {
res.html(
renderPlaygroundPage({
endpoint: `/our/graphql`,
}),
)
next()
})
```

with `expressPlayground`:

```js
// params
app.get('/playground', (req) =>
expressPlayground({
endpoint: `/our/graphql`,
settings: { 'editor.theme': req.query.darkMode ? 'dark' : 'light' },
}),
)
```

with `koaPlayground`:

```js
const koa = require('koa')
const koaRouter = require('koa-router')
const koaPlayground = require('graphql-playground-middleware-koa')

const app = new koa()
const router = new koaRouter()

router.all('/playground', koaPlayground({ endpoint: '/graphql' }))
```

### Vulnerable Examples

Here are some examples where the vulnerability would be present before the patch, because of unfiltered user input

```js
const express = require('express')
const expressPlayground = require('graphql-playground-middleware-express')
.default

const app = express()

app.use(express.json())

// params
app.get('/playground/:id', (req) =>
expressPlayground({
endpoint: `/our/graphql/${req.params.id}`,
}),
)

// params
app.get('/playground', (req) =>
expressPlayground({
endpoint: `/our/graphql`,
// any settings that are unsanitized user input, not just `endpoint`
settings: { 'editor.fontFamily': req.query.font },
}),
)
```

### Workaround

To fix this issue without the update, you can sanitize however you want.

We suggest using [`xss`](https://www.npmjs.com/package/xss) (what we use for our own fix)

For example, with `graphql-playground-middleware-express`:

```js
const express = require('express')
const { filterXSS } = require('xss')
const expressPlayground = require('graphql-playground-middleware-express')
.default


const app = express()

const filter = (val) => filterXSS(val, {
whitelist: [],
stripIgnoreTag: true,
stripIgnoreTagBody: ['script']
})

// simple example
app.get('/playground/:id', (req) =>
expressPlayground({ endpoint: `/graphql/${filter(req.params.id)}` })

// advanced params
app.get('/playground', (req) =>
expressPlayground(JSON.parse(filter(JSON.stringify(req.query))))
```
2 changes: 2 additions & 0 deletions packages/graphql-playground-html/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# graphql-playground-html

> **SECURITY WARNING:** This package and all of it's dependendents had a severe XSS Reflection attack vulnerability until version `1.6.20` of this package. You must sanitize any and all user input values to `renderPlaygroundPage()` values. If you used static values in your middlewares, including ours, you were not vulnerable to the attack.
This package is being used by the GraphQL Playground middlewares.

For local development, you can `yarn link` this package, then use `yarn link graphql-playground-html` in the
Expand Down
4 changes: 3 additions & 1 deletion packages/graphql-playground-html/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,7 @@
"typescript": {
"definition": "dist/index.d.ts"
},
"dependencies": {}
"dependencies": {
"xss": "^1.0.6"
}
}
Loading

0 comments on commit bf1883d

Please sign in to comment.