DiscordMessenger / dm

Discord Messenger is a free Discord-compatible messaging client targeting both new and old Windows.
MIT License
862 stars 16 forks source link

SECURITY ADVISORY: Discord Messenger does not check the server's certificate #45

Closed iProgramMC closed 4 months ago

iProgramMC commented 4 months ago

Hello everyone.

I have just realized that me disabling server verification (src/windows/NetworkerThread.cpp) leads to the possibility of a man in the middle attack.

If using V1.00, ensure that you can access Discord and don't get certificate errors while connected to the network you're trying to use Discord Messenger from.

This will be fixed in a future version by bundling the certificate(s) for Discord into the client.

iProgramMC commented 4 months ago

More precisely, this assumption was fine back when Discord Messenger was still being developed, but not now. https://github.com/DiscordMessenger/dm/blob/master/src/windows/NetworkerThread.cpp#L107

iProgramMC commented 4 months ago

The plan is to export the Baltimore CyberTrust Root certificate and embed it into the executable as a resource. Then it'd be written to the cache and loaded by httplib.

image

iProgramMC commented 4 months ago

To fix this, I ought to make httplib load the certificates from Discord. I don't know how to do that, so I'll do the next best thing - enable server verification by default and disable it if required to log in.

iProgramMC commented 4 months ago

NOTE: HTTPS and Websocket traffic is always encrypted. However, disabling server verification puts you at risk of accessing a compromised service.

iProgramMC commented 4 months ago

Ok, seems like I will also need to get websocketpp to work. Tried adding these to the tls init handler but it won't even connect to the websocket gateway on Win11 due to TLS handshake errors:

ctx->set_default_verify_paths();
ctx->set_verify_mode(websocketpp::lib::asio::ssl::verify_peer | websocketpp::lib::asio::ssl::verify_fail_if_no_peer_cert);
Zero3K commented 4 months ago

I think this issue should be unpinned since it has been fixed.

iProgramMC commented 4 months ago

True. There isn't a need to remind me though.