engine.io icon indicating copy to clipboard operation
engine.io copied to clipboard
verified publisher icon socketio

Read sid from header if not provided in query

Open gasnier opened this issue 6 years ago • 5 comments

The kind of change this PR does introduce

  • [ ] a bug fix
  • [ ] a new feature
  • [ ] an update to the documentation
  • [ ] a code change that improves performance
  • [x] other

Current behaviour

For now, the sid is sent in the GET request parameters. OWASP Zed Attack Proxy says there is a security issue See https://github.com/socketio/socket.io/issues/3416

New behaviour

If the sid is not provided in the GET request parameters, we try to find it in the headers

Other information (e.g. related issues)

I will also propose a pull request in the socket.io repository so that the client sends the sid in the header.

gasnier avatar May 02 '19 17:05 gasnier

Is there a chance that my pull requests get merged ?

gasnier avatar Jun 10 '19 12:06 gasnier

@gasnier hi, thanks for the pull request! A few thoughts:

  • I'm not sure about the name of the header. Shouldn't it be prefixed by x-? Something like x-sid maybe?
  • could you add some tests please?

darrachequesne avatar Jun 18 '19 06:06 darrachequesne

  • I'm not sure about the name of the header. Shouldn't it be prefixed by x-? Something like x-sid maybe? Ok to change to x-sid
  • could you add some tests please? I will have a look to the current tests to see where I can add one...

gasnier avatar Jun 19 '19 18:06 gasnier

@darrachequesne Could you merge this pull request in soon? We have to fix the vulnerability of "Session ID sent in URL Rewrite".

yangpu avatar Oct 16 '19 04:10 yangpu

Please see my comment here: https://github.com/socketio/engine.io-client/pull/610#issuecomment-638743774

darrachequesne avatar Jun 04 '20 11:06 darrachequesne