Nuxt security with Socket.io and xssValidator make POST requests hanging

[noook]

Version

nuxt-security: 2.0.0-beta.5
nuxt: 3.11.2

Reproduction Link

https://github.com/noook/nuxt-security-socket-io-repro

Steps to reproduce

1. Run the project linked
2. Open devtools, network tab
3. Click the "Emit event button"
4. Observe POST requests hanging

What is Expected?

Successful connection to socket.io server and successful requests.

• When disabling xssValidator option, it works.
• when enabling the option, but with only GET requests it works as well.

What is actually happening?

POST requests never end.

Notes

I don't know if those options are required for the project to work

security: {
  contentSecurityPolicy: {
    'connect-src': [
      '\'self\'',
      'ws:',
      'wss:',
    ],
  },
}

[Baroshem]

Hey,

Thanks for reporting this issue. XSS validator works for both Get and Post requests so probably it is cataching the request and cannot parse it properly. Do you maybe have access to the logs of your app to see if it says something in the console?

[noook]

There is no relevant log regarding this issue. I already digged in your module and tried to do some early returns in the xssValidator middleware and everything seems "ok", I can't figure out what's happening. It reaches properly the end of the middleware with no error inbetween.

Can you reproduce with my link ? I tried to provide a reproduction as minimal as possible, with the alternatives where it is working

[noook]

Now that I think about it, I think I noticed the error occurred around here when debugging:

(Notice the && false for escaping the middleware)

// OK
if (rules.xssValidator.methods && rules.xssValidator.methods.includes(event.node.req.method) && false) {

// ...
const valueToFilter = event.node.req.method === "GET"
  ? getQuery(event)
  : event.node.req.headers["content-type"]?.includes("multipart/form-data")
    ? await readMultipartFormData(event)
    : await readBody(event);

// Escaping here makes the POST request hanging, maybe is it related to the above line ?
if (valueToFilter && Object.keys(valueToFilter).length && false) {

[Baroshem]

Thanks for the additional digging!

Yes, it could be related to that. Would you be interested in creating a Pull Request with a fix for that? I think we would need to support a new case of sockets that wasn't handled before as we were working with pure http requests with body, then we added the multipart form data and today we would also need to support a new case of sockets :)

I could provide all the help needed 🚀

[GalacticHypernova]

Just tried it through stackblitz, can't seem to reproduce it:

I made sure to use the module as is, without disabling xss. Was only able to reproduce it under one condition which appears to be completely unrelated, will investigate some more.

Update: turns out it was an edge case that was indeed related to it, will test the fix locally before making a PR

@noook do you allow me to use your repro as a test case?

[noook]

Of course! My repository exists for this very exact reason

[GalacticHypernova]

@noook @Baroshem a little update on this:
I managed to pinpoint the issue and it is unrelated to the module, but rather directly in h3, so in here nothing needs to be changed unless we want to make a custom method of parsing request body, as the issue is in the readRawBody function.
The section at fault is the promise that is built to listen for error, data, and end of the request:

const promise = ((event.node.req as any)[RawBodySymbol] = new Promise<Buffer>(
    (resolve, reject) => {
      const bodyData: any[] = [];
      event.node.req
        .on('error', (err) => {
          reject(err);
        })
        .on('data', (chunk) => {
          bodyData.push(chunk);
        })
        .on('end', () => {
          resolve(Buffer.concat(bodyData));
        });
    }
  ))

[noook]

Mentioning the code with this permalink:

https://github.com/unjs/h3/blob/58e33ff00b1db1cf86a780bf8b152c3f7e573350/src/utils/body.ts#L104-L118

Are you suggesting a fix would be to store the result of the promise in a variable attached to the event, so that it can be read again later on ?

I could open an issue for that and eventually provide the fix.

[GalacticHypernova]

Well, the issue is not really there, it's in the second promise with bodyData array. I'm experimenting with it to figure out how this could work.
It honestly looks like a language limitation. on seems to break it only for that one request, I'll try to look into how to read it without it.
I think for the time being though it should be opened as an issue in the h3 repo. I managed to track it down to specifically the on('data') for the request, and I don't see anything unusual about it.

[Baroshem]

Great @GalacticHypernova that you have found it. I think you could talk with Pooya about it :)

Should we close this issue as it is an upstream?

[GalacticHypernova]

I think we should, though it's definitely interesting why websockets (or at least this specific websocket) can't use data to read. Ideally it shouldn't do it. I'm not even sure it is an h3 issue. Theoretically it could be an issue of the websocket library itself, I'll try a different websocket and see