Clients can read messages from all rooms, even if not joined to rooms

[shaldengeki]

Hi there!

I happened across your project recently (love the idea!), and while reading through the code I think I uncovered a security hole.

Let's say you have a client A in a private channel A'. When A sends a message to the server, it sends the room (A') that the message is associated with to the server as a field on the payload:
https://github.com/astrosonic/sanctuary/blob/master/templates/actiroom.html#L84-L94

The server checks to see if the room is active, and then broadcasts that message out to all connected clients:
https://github.com/astrosonic/sanctuary/blob/master/main.py#L168-L171

So if we have a client B in a separate private channel B', they will receive the message. Currently this isn't noticeable if B is a casual observer, because only messages from the joined room get displayed:
https://github.com/astrosonic/sanctuary/blob/master/templates/actiroom.html#L108

However, all B has to do to read A's messages is to remove that filtering out; this is trivial to do in a browser's Javascript console. This means that any malicious client with network access to the chat server can listen in on all the messages being sent on the server in any room.

To fix this, I think you'd want to do two things:

• Server-side, use SocketIO's built-in room support to send messages only to clients within that room
• Server-side, only allow clients to connect (to read or publish messages) if they've supplied the correct password for the room

Does that make sense? Happy to clarify!

[gridhead]

Oh boy. That sure is a huge vulnerability. Thanks for letting us know about it. I would surely get to work with it. Plus, the code is open-source right now so if you believe you can fix the issue, feel free to fork the repository and make a pull request with the changes. 😄

[gridhead]

Sanctuary has been rewritten and this issue might be irrelevant now. I am still keeping this open and won't close it until I'm 100% sure that the issue specified herewith does not persist.