From eabfbab403e6ba10219c262518899db5a60d0c1e Mon Sep 17 00:00:00 2001 From: Lucas Toole Date: Tue, 2 Mar 2021 08:24:29 -0600 Subject: [PATCH] FIX(Server): Unauthenticated connections no longer add to user count This commit fixes both issues discussed in #4277 which were related to the fact that any socket connection was allocated an ID and marked as a user. This bug allowed for a malicious user to lock out all users of a server regardless of server password usage. This change simply moves ID allocation to after a connection is marked as Authenticated. FIXES: #4277 --- src/murmur/Messages.cpp | 3 +++ src/murmur/Server.cpp | 7 ------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/murmur/Messages.cpp b/src/murmur/Messages.cpp index a073a0933..0a482c7fa 100644 --- a/src/murmur/Messages.cpp +++ b/src/murmur/Messages.cpp @@ -379,7 +379,10 @@ void Server::msgAuthenticate(ServerUser *uSource, MumbleProto::Authenticate &msg { QWriteLocker wl(&qrwlVoiceThread); + uSource->uiSession = qqIds.dequeue(); uSource->sState = ServerUser::Authenticated; + qhUsers.insert(uSource->uiSession, uSource); + qhHostUsers[uSource->haAddress].insert(uSource); } mpus.set_session(uSource->uiSession); diff --git a/src/murmur/Server.cpp b/src/murmur/Server.cpp index 4122e975b..a1ea19ccc 100644 --- a/src/murmur/Server.cpp +++ b/src/murmur/Server.cpp @@ -1450,16 +1450,9 @@ void Server::newClient() { } ServerUser *u = new ServerUser(this, sock); - u->uiSession = qqIds.dequeue(); u->haAddress = ha; HostAddress(sock->localAddress()).toSockaddr(&u->saiTcpLocalAddress); - { - QWriteLocker wl(&qrwlVoiceThread); - qhUsers.insert(u->uiSession, u); - qhHostUsers[ha].insert(u); - } - connect(u, SIGNAL(connectionClosed(QAbstractSocket::SocketError, const QString &)), this, SLOT(connectionClosed(QAbstractSocket::SocketError, const QString &))); connect(u, SIGNAL(message(unsigned int, const QByteArray &)), this,