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

Websocket hijacking vulnerability #128

Closed
StoneMoe opened this issue Jul 14, 2019 · 11 comments

Comments

@StoneMoe
Copy link
Contributor

commented Jul 14, 2019

CORS headers are only works in XHR requests, and ignored by clients during a websocket connection.

Ref: https://tools.ietf.org/html/rfc6455#section-4.2.2

 /origin/
          The |Origin| header field in the client's handshake indicates
          the origin of the script establishing the connection.  The
          origin is serialized to ASCII and converted to lowercase.  The
          server MAY use this information as part of a determination of
          whether to accept the incoming connection.  If the server does
          not validate the origin, it will accept connections from
          anywhere.  If the server does not wish to accept this
          connection, it MUST return an appropriate HTTP error code
          (e.g., 403 Forbidden) and abort the WebSocket handshake
          described in this section.  For more detail, refer to
          Section 10.

TLDR; If the server does not wish to accept this connection, it MUST return an appropriate HTTP error code (e.g., 403 Forbidden) and abort the WebSocket handshake

@miguelgrinberg

This comment has been minimized.

Copy link
Owner

commented Jul 14, 2019

Can you explain in detail what is the exploit that you found?

@StoneMoe

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2019

Sorry for the lack of details.

How it works

Assuming we are hosting a crafted HTML Page at a.com as attacker, when victim visit our page, we can initiate a websocket connection directly to b.com/socket.io using victim's credential with no limitation.

It works like CSRF, but in websocket world it called Cross-Site WebSocket Hijacking (CSWSH)

Why

Websocket connections not bound by CORS, and python-engineio only used CORS headers for rejecting invalid Origin connections

https://github.com/miguelgrinberg/python-engineio/blob/master/engineio/server.py#L575

Fix

Abort invalid Origin request for websocket connections like the description at the beginning

@StoneMoe

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

Here is a temporary fix:

@socket_io.on('connect', namespace='/ns')
def on_client_connect():
    if request.headers.get('Origin', 'invalid') not in socket_io.server_options.get('cors_allowed_origins', []):
        logging.warning('WebSocket Hijacking detected from domain "%s"' % request.headers.get('Origin', 'invalid'))
        disconnect()  # return False in connect handler not working?
        return False

And we can see the hijacking attempts that from https://amritb.github.io/socketio-client-tool are not working now.

web_1     | [2019-07-15 18:26:18][root      ][WARNING] WebSocket Hijacking detected from domain "https://amritb.github.io"
@miguelgrinberg

This comment has been minimized.

Copy link
Owner

commented Jul 15, 2019

Okay, makes sense, thanks.

@miguelgrinberg miguelgrinberg added enhancement and removed question labels Jul 15, 2019

@miguelgrinberg miguelgrinberg self-assigned this Jul 15, 2019

@nluedtke

This comment has been minimized.

Copy link

commented Jul 16, 2019

This was assigned CVE-2019-13611.

@benSlaughter

This comment has been minimized.

Copy link

commented Jul 25, 2019

Hi, I work on GitHub's security workflows team, and we are currently evaluating CVE-2019-13611 as it is in our holding pattern for publishing. We would like to reach out to you, and get some feedback as we plan to publish this soon, however we would like to ensure that we are responsibly disclosing this information.
Is this a legitimate security vulnerability, and if so is there a patch in progress?

@miguelgrinberg

This comment has been minimized.

Copy link
Owner

commented Jul 25, 2019

Hi @benSlaughter.

This is basically a vulnerability in the WebSocket protocol, not in this package. The actual issue is the lack of CORS controls for WebSocket connections in browsers. This package uses several WebSocket implementations as optional dependencies that allow clients to establish WebSocket connections, so someone has decided that this package needs to do this validation that browsers don't do.

I have been thinking about this to see if there is anything I can do from my side to address this in a way that is generic and works with all the WebSocket server implementations I support, but I honestly feel this should be better done in each WebSocket implementation.

@miguelgrinberg

This comment has been minimized.

Copy link
Owner

commented Jul 28, 2019

I believe I have addressed this problem by implementing the recommended verification of the origin header when present in incoming requests.

Unfortunately rolling out this fix will take a bit of orchestration on my part as I want to minimize breaking people's code. There are a couple other packages that depend on this one that will also need their versions and dependencies bumped like this one to make it clear that upgrading could be potentially backwards incompatible. I expect the fix will be rolled out later in the week.

miguelgrinberg added a commit that referenced this issue Jul 28, 2019

@benSlaughter

This comment has been minimized.

Copy link

commented Jul 29, 2019

Hi @miguelgrinberg thanks so much for getting back to me.
In that case would you be happy to create a Security Advisory for this and publish it once you have released a fixed version?
Our tooling will pick that advisory up once you publish it and we’ll send alerts to your users including the details you write there, along with details of the fixed version.
I’ve put a one week hold on this on our side in the meantime.

@miguelgrinberg

This comment has been minimized.

Copy link
Owner

commented Jul 29, 2019

@benSlaughter how will you know which version(s) are affected? The advisory does not have any fields to indicate the version range. Thanks.

@benSlaughter

This comment has been minimized.

Copy link

commented Jul 29, 2019

Hi @miguelgrinberg
Once a draft advisory has been opened, there is a form that you can fill in under "Required advisory information" that includes fields for affected versions and patched versions.
Here are the docs, hope that helps.
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.