From af09d0d2949f2dd134437713a83a672c4c7027be Mon Sep 17 00:00:00 2001 From: Daenyth Date: Wed, 25 Jul 2012 00:08:28 -0400 Subject: [PATCH] Reject more invalid usernames from clients. Specifically this should cover people connecting with a username of "\u200C" --- cockatrice/src/window_main.cpp | 4 ++++ common/pb/event_connection_closed.proto | 1 + common/pb/response.proto | 1 + common/server.cpp | 2 +- common/server.h | 2 +- common/server_protocolhandler.cpp | 11 ++++++----- .../src/servatrice_database_interface.cpp | 18 ++++++++++++++++++ servatrice/src/servatrice_database_interface.h | 1 + 8 files changed, 33 insertions(+), 7 deletions(-) diff --git a/cockatrice/src/window_main.cpp b/cockatrice/src/window_main.cpp index a521cec8..4cccf1d0 100644 --- a/cockatrice/src/window_main.cpp +++ b/cockatrice/src/window_main.cpp @@ -76,6 +76,7 @@ void MainWindow::processConnectionClosedEvent(const Event_ConnectionClosed &even break; } case Event_ConnectionClosed::SERVER_SHUTDOWN: reasonStr = tr("Scheduled server shutdown."); break; + case Event_ConnectionClosed::USERNAMEINVALID: reasonStr = tr("Invalid username."); break; default: reasonStr = QString::fromStdString(event.reason_str()); } QMessageBox::critical(this, tr("Connection closed"), tr("The server has terminated your connection.\nReason: %1").arg(reasonStr)); @@ -257,6 +258,9 @@ void MainWindow::loginError(Response::ResponseCode r, QString reasonStr, quint32 QMessageBox::critical(this, tr("Error"), bannedStr); break; } + case Response::RespUserIsBanned: + QMessageBox::critical(this, tr("Error"), tr("Invalid username.")); + break; default: QMessageBox::critical(this, tr("Error"), tr("Unknown login error: %1").arg(static_cast(r))); } diff --git a/common/pb/event_connection_closed.proto b/common/pb/event_connection_closed.proto index 55ea954f..e516f172 100644 --- a/common/pb/event_connection_closed.proto +++ b/common/pb/event_connection_closed.proto @@ -9,6 +9,7 @@ message Event_ConnectionClosed { SERVER_SHUTDOWN = 2; TOO_MANY_CONNECTIONS = 3; BANNED = 4; + USERNAMEINVALID = 5; } optional CloseReason reason = 1; optional string reason_str = 2; diff --git a/common/pb/response.proto b/common/pb/response.proto index 7e1431fa..4fa3c447 100644 --- a/common/pb/response.proto +++ b/common/pb/response.proto @@ -22,6 +22,7 @@ message Response { RespChatFlood = 18; RespUserIsBanned = 19; RespAccessDenied = 20; + RespUsernameInvalid = 21; } enum ResponseType { JOIN_ROOM = 1000; diff --git a/common/server.cpp b/common/server.cpp index 32f3c878..149805f2 100644 --- a/common/server.cpp +++ b/common/server.cpp @@ -114,7 +114,7 @@ AuthenticationResult Server::loginUser(Server_ProtocolHandler *session, QString QWriteLocker locker(&clientsLock); AuthenticationResult authState = databaseInterface->checkUserPassword(session, name, password, reasonStr, secondsLeft); - if ((authState == NotLoggedIn) || (authState == UserIsBanned)) + if ((authState == NotLoggedIn) || (authState == UserIsBanned || authState == UsernameInvalid)) return authState; ServerInfo_User data = databaseInterface->getUserData(name, true); diff --git a/common/server.h b/common/server.h index aeb11bad..a09eb31c 100644 --- a/common/server.h +++ b/common/server.h @@ -27,7 +27,7 @@ class GameEventContainer; class CommandContainer; class Command_JoinGame; -enum AuthenticationResult { NotLoggedIn = 0, PasswordRight = 1, UnknownUser = 2, WouldOverwriteOldSession = 3, UserIsBanned = 4 }; +enum AuthenticationResult { NotLoggedIn = 0, PasswordRight = 1, UnknownUser = 2, WouldOverwriteOldSession = 3, UserIsBanned = 4, UsernameInvalid = 5 }; class Server : public QObject { diff --git a/common/server_protocolhandler.cpp b/common/server_protocolhandler.cpp index c4a9e00a..5054bbc7 100644 --- a/common/server_protocolhandler.cpp +++ b/common/server_protocolhandler.cpp @@ -200,11 +200,11 @@ Response::ResponseCode Server_ProtocolHandler::processGameCommandContainer(const if (!game) { if (room->getExternalGames().contains(cont.game_id())) { server->sendIsl_GameCommand(cont, - room->getExternalGames().value(cont.game_id()).server_id(), - userInfo->session_id(), - roomIdAndPlayerId.first, - roomIdAndPlayerId.second - ); + room->getExternalGames().value(cont.game_id()).server_id(), + userInfo->session_id(), + roomIdAndPlayerId.first, + roomIdAndPlayerId.second + ); return Response::RespNothing; } return Response::RespNotInRoom; @@ -338,6 +338,7 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd } case NotLoggedIn: return Response::RespWrongPassword; case WouldOverwriteOldSession: return Response::RespWouldOverwriteOldSession; + case UsernameInvalid: return Response::RespUsernameInvalid; default: authState = res; } diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 15e7084f..9ef6f094 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -73,6 +73,21 @@ bool Servatrice_DatabaseInterface::execSqlQuery(QSqlQuery &query) return false; } +bool Servatrice_DatabaseInterface::usernameIsValid(const QString &user) +{ + QString result; + result.reserve(user.size()); + foreach (const QChar& c, user) { + switch (c.category()) { + // TODO: Figure out exactly which categories are OK and not + case QChar::Other_Control: break; + default: result += c; + } + } + result = result.trimmed(); + return (result.size() > 0); +} + AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_ProtocolHandler *handler, const QString &user, const QString &password, QString &reasonStr, int &banSecondsLeft) { switch (server->getAuthenticationMethod()) { @@ -80,6 +95,9 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot case Servatrice::AuthenticationSql: { if (!checkSql()) return UnknownUser; + + if (!usernameIsValid(user)) + return UsernameInvalid; QSqlQuery ipBanQuery(sqlDatabase); ipBanQuery.prepare("select time_to_sec(timediff(now(), date_add(b.time_from, interval b.minutes minute))), b.minutes <=> 0, b.visible_reason from " + server->getDbPrefix() + "_bans b where b.time_from = (select max(c.time_from) from " + server->getDbPrefix() + "_bans c where c.ip_address = :address) and b.ip_address = :address2"); diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index d9bd856a..814432cc 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -16,6 +16,7 @@ private: QSqlDatabase sqlDatabase; Servatrice *server; ServerInfo_User evalUserQueryResult(const QSqlQuery &query, bool complete, bool withId = false); + bool usernameIsValid(const QString &user); protected: AuthenticationResult checkUserPassword(Server_ProtocolHandler *handler, const QString &user, const QString &password, QString &reasonStr, int &secondsLeft); public slots: