Skip to content

Commit bf1883d

Browse files
authored
Merge pull request from GHSA-4852-vrh7-28rf
* fix: security vulnerability allowing XSS reflection with user input * fix: update docs, replace santize-html and sanitizeUrl with xss module
1 parent 40c35fc commit bf1883d

File tree

14 files changed

+447
-92
lines changed

14 files changed

+447
-92
lines changed

README.md

+75-43
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
> **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)
2+
13
<p align="center"><img src="https://imgur.com/5fzMbyV.png" width="269"></p>
24

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

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

30+
## Security Details
31+
32+
**NOTE: only _unsanitized user input_ to the functions in these packages is vulnerable** to the recently reported XSS Reflection attack.
33+
34+
### Impact
35+
36+
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.
37+
38+
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.
39+
40+
### Impacted Packages
41+
42+
**All versions of these packages are impacted until the ones specified below**, which are now safe for user defined input:
43+
44+
- `graphql-playground-html`: **☔ safe** @ `1.6.20`
45+
- `graphql-playground-express` **☔ safe** @ `1.7.15`
46+
- `graphql-playground-koa` **☔ safe** @ `1.6.14`
47+
- `graphql-playground-hapi` **☔ safe** @ `1.6.12`
48+
- `graphql-playground-lambda` **☔ safe** @ `1.7.16`
49+
- `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)
50+
- `graphql-playground-react` is safe because it does not use `renderPlaygroundPage()` anywhere, and thus is not susceptible to template string XSS reflection attacks.
51+
52+
### More Information
53+
54+
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.
55+
2856
## FAQ
2957

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

123151
```ts
124152
interface Tab {
125-
endpoint: string
126-
query: string
127-
name?: string
128-
variables?: string
129-
responses?: string[]
130-
headers?: { [key: string]: string }
153+
endpoint: string
154+
query: string
155+
name?: string
156+
variables?: string
157+
responses?: string[]
158+
headers?: { [key: string]: string }
131159
}
132160
```
133161

@@ -169,8 +197,8 @@ Including Fonts (`1.`)
169197

170198
```html
171199
<link
172-
href="https://fonts.googleapis.com/css?family=Open+Sans:300,400,600,700|Source+Code+Pro:400,700"
173-
rel="stylesheet"
200+
href="https://fonts.googleapis.com/css?family=Open+Sans:300,400,600,700|Source+Code+Pro:400,700"
201+
rel="stylesheet"
174202
/>
175203
```
176204

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

185213
ReactDOM.render(
186-
<Provider store={store}>
187-
<Playground endpoint="https://api.graph.cool/simple/v1/swapi" />
188-
</Provider>,
189-
document.body,
214+
<Provider store={store}>
215+
<Playground endpoint='https://api.graph.cool/simple/v1/swapi' />
216+
</Provider>,
217+
document.body,
190218
)
191219
```
192220

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

234262
exports.graphqlHandler = function graphqlHandler(event, context, callback) {
235-
function callbackFilter(error, output) {
236-
// eslint-disable-next-line no-param-reassign
237-
output.headers['Access-Control-Allow-Origin'] = '*'
238-
callback(error, output)
239-
}
240-
241-
const handler = graphqlLambda({ schema: myGraphQLSchema })
242-
return handler(event, context, callbackFilter)
263+
function callbackFilter(error, output) {
264+
// eslint-disable-next-line no-param-reassign
265+
output.headers['Access-Control-Allow-Origin'] = '*'
266+
callback(error, output)
267+
}
268+
269+
const handler = graphqlLambda({ schema: myGraphQLSchema })
270+
return handler(event, context, callbackFilter)
243271
}
244272

245273
exports.playgroundHandler = lambdaPlayground({
246-
endpoint: '/dev/graphql',
274+
endpoint: '/dev/graphql',
247275
})
248276
```
249277

@@ -267,6 +295,10 @@ functions:
267295
cors: true
268296
```
269297
298+
#### Security Issue
299+
300+
There is an [XSS Reflection Vulnerability](./SECURITY.md) when using these middlewares with unsanitized user input before
301+
270302
## Development
271303
272304
```sh
@@ -285,27 +317,27 @@ These are the available options:
285317

286318
```ts
287319
export interface EditorColours {
288-
property: string
289-
comment: string
290-
punctuation: string
291-
keyword: string
292-
def: string
293-
qualifier: string
294-
attribute: string
295-
number: string
296-
string: string
297-
builtin: string
298-
string2: string
299-
variable: string
300-
meta: string
301-
atom: string
302-
ws: string
303-
selection: string
304-
cursorColor: string
305-
editorBackground: string
306-
resultBackground: string
307-
leftDrawerBackground: string
308-
rightDrawerBackground: string
320+
property: string
321+
comment: string
322+
punctuation: string
323+
keyword: string
324+
def: string
325+
qualifier: string
326+
attribute: string
327+
number: string
328+
string: string
329+
builtin: string
330+
string2: string
331+
variable: string
332+
meta: string
333+
atom: string
334+
ws: string
335+
selection: string
336+
cursorColor: string
337+
editorBackground: string
338+
resultBackground: string
339+
leftDrawerBackground: string
340+
rightDrawerBackground: string
309341
}
310342
```
311343

SECURITY.md

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
# Known Vulnerabilities
2+
3+
## XSS Reflection Vulnerability
4+
5+
the origin of the vulnerability is in `renderPlaygroundPage`, found in `graphql-playground-html`
6+
7+
### Impact
8+
9+
When using
10+
11+
- `renderPlaygroundPage()`,
12+
- `koaPlayground()`
13+
- `expressPlayground()`
14+
- `koaPlayground()`
15+
- `lambdaPlayground()`
16+
- any downstream dependents that use these functions
17+
18+
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.
19+
20+
### Impacted Packages
21+
22+
**All versions of these packages are impacted until those specified below**, which are now safe for user defined input:
23+
24+
- `graphql-playground-html`: **☔ safe** @ `1.6.20`
25+
- `graphql-playground-express` **☔ safe** @ `1.7.15`
26+
- `graphql-playground-koa` **☔ safe** @ `1.6.14`
27+
- `graphql-playground-hapi` **☔ safe** @ `1.6.12`
28+
- `graphql-playground-lambda` **☔ safe** @ `1.7.16`
29+
30+
### Static input was always safe
31+
32+
These examples are safe for _all versions_ **because input is static**
33+
34+
with `express` and `renderPlaygroundPage`:
35+
36+
```js
37+
app.get('/playground', (req) => {
38+
res.html(
39+
renderPlaygroundPage({
40+
endpoint: `/our/graphql`,
41+
}),
42+
)
43+
next()
44+
})
45+
```
46+
47+
with `expressPlayground`:
48+
49+
```js
50+
// params
51+
app.get('/playground', (req) =>
52+
expressPlayground({
53+
endpoint: `/our/graphql`,
54+
settings: { 'editor.theme': req.query.darkMode ? 'dark' : 'light' },
55+
}),
56+
)
57+
```
58+
59+
with `koaPlayground`:
60+
61+
```js
62+
const koa = require('koa')
63+
const koaRouter = require('koa-router')
64+
const koaPlayground = require('graphql-playground-middleware-koa')
65+
66+
const app = new koa()
67+
const router = new koaRouter()
68+
69+
router.all('/playground', koaPlayground({ endpoint: '/graphql' }))
70+
```
71+
72+
### Vulnerable Examples
73+
74+
Here are some examples where the vulnerability would be present before the patch, because of unfiltered user input
75+
76+
```js
77+
const express = require('express')
78+
const expressPlayground = require('graphql-playground-middleware-express')
79+
.default
80+
81+
const app = express()
82+
83+
app.use(express.json())
84+
85+
// params
86+
app.get('/playground/:id', (req) =>
87+
expressPlayground({
88+
endpoint: `/our/graphql/${req.params.id}`,
89+
}),
90+
)
91+
92+
// params
93+
app.get('/playground', (req) =>
94+
expressPlayground({
95+
endpoint: `/our/graphql`,
96+
// any settings that are unsanitized user input, not just `endpoint`
97+
settings: { 'editor.fontFamily': req.query.font },
98+
}),
99+
)
100+
```
101+
102+
### Workaround
103+
104+
To fix this issue without the update, you can sanitize however you want.
105+
106+
We suggest using [`xss`](https://www.npmjs.com/package/xss) (what we use for our own fix)
107+
108+
For example, with `graphql-playground-middleware-express`:
109+
110+
```js
111+
const express = require('express')
112+
const { filterXSS } = require('xss')
113+
const expressPlayground = require('graphql-playground-middleware-express')
114+
.default
115+
116+
117+
const app = express()
118+
119+
const filter = (val) => filterXSS(val, {
120+
whitelist: [],
121+
stripIgnoreTag: true,
122+
stripIgnoreTagBody: ['script']
123+
})
124+
125+
// simple example
126+
app.get('/playground/:id', (req) =>
127+
expressPlayground({ endpoint: `/graphql/${filter(req.params.id)}` })
128+
129+
// advanced params
130+
app.get('/playground', (req) =>
131+
expressPlayground(JSON.parse(filter(JSON.stringify(req.query))))
132+
```

packages/graphql-playground-html/README.md

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# graphql-playground-html
22

3+
> **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.
4+
35
This package is being used by the GraphQL Playground middlewares.
46

57
For local development, you can `yarn link` this package, then use `yarn link graphql-playground-html` in the

packages/graphql-playground-html/package.json

+3-1
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,7 @@
3333
"typescript": {
3434
"definition": "dist/index.d.ts"
3535
},
36-
"dependencies": {}
36+
"dependencies": {
37+
"xss": "^1.0.6"
38+
}
3739
}

0 commit comments

Comments
 (0)