From 25d57ce825db044ab628131ca0a566ad9c218701 Mon Sep 17 00:00:00 2001 From: WolverinDEV Date: Wed, 28 Apr 2021 18:32:00 +0200 Subject: [PATCH] Properly counting group name lengths and fixed a crash on server create when the group creation process failed --- server/src/channel/ServerChannel.cpp | 5 ++-- server/src/client/SpeakingClient.cpp | 12 ++++---- server/src/client/command_handler/channel.cpp | 5 ++-- server/src/client/command_handler/client.cpp | 29 ++++++++++++++----- server/src/groups/GroupManager.cpp | 13 ++++++--- 5 files changed, 44 insertions(+), 20 deletions(-) diff --git a/server/src/channel/ServerChannel.cpp b/server/src/channel/ServerChannel.cpp index e8a5816..a30d8da 100644 --- a/server/src/channel/ServerChannel.cpp +++ b/server/src/channel/ServerChannel.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include "misc/rnd.h" #include "src/VirtualServer.h" #include "src/client/ConnectedClient.h" @@ -426,8 +427,8 @@ bool ServerChannelTree::validateChannelNames() { } */ - for(const auto &channel : this->channels()){ - auto name_length = count_characters(channel->name()); + for(const auto &channel : this->channels()) { + auto name_length = utf8::count_characters(channel->name()); if(name_length > 40) { logError(this->getServerId(), "Channel {} loaded an invalid name from the database (name to long). Cutting channel name"); channel->properties()[property::CHANNEL_NAME] = channel->name().substr(0, 40); //FIXME count UTF8 diff --git a/server/src/client/SpeakingClient.cpp b/server/src/client/SpeakingClient.cpp index ef3646b..bde353b 100644 --- a/server/src/client/SpeakingClient.cpp +++ b/server/src/client/SpeakingClient.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include "src/channel/ClientChannelView.h" #include "SpeakingClient.h" #include "src/InstanceHandler.h" @@ -100,20 +101,21 @@ command_result SpeakingClient::applyClientInitParameters(Command &cmd) { /* That's ok */ continue; } else if(key == "client_nickname") { - auto name = cmd[key].string(); - if (count_characters(name) < 3) { + auto name_length = utf8::count_characters(cmd[key].string()); + if (name_length < 3) { return command_result{error::parameter_invalid, "client_nickname"}; } - if (count_characters(name) > 30) { + if (name_length > 30) { return command_result{error::parameter_invalid, "client_nickname"}; } /* The unique of the name will be checked when registering the client at the target server */ - this->properties()[property::CLIENT_NICKNAME] = name; + this->properties()[property::CLIENT_NICKNAME] = cmd[key].string(); } else if(key == "client_nickname_phonetic") { auto name = cmd["client_nickname_phonetic"].string(); - if (count_characters(name) > 30) { + auto name_length = utf8::count_characters(name); + if (name_length < 0 || name_length > 30) { return command_result{error::parameter_invalid, "client_nickname_phonetic"}; } this->properties()[property::CLIENT_NICKNAME_PHONETIC] = name; diff --git a/server/src/client/command_handler/channel.cpp b/server/src/client/command_handler/channel.cpp index d4f6771..b5800fd 100644 --- a/server/src/client/command_handler/channel.cpp +++ b/server/src/client/command_handler/channel.cpp @@ -25,6 +25,7 @@ #include #include #include +#include using namespace std::chrono; using namespace std; @@ -886,8 +887,8 @@ ts::command_result ConnectedClient::execute_channel_edit(ChannelId& channel_id, case property::CHANNEL_NAME: case property::CHANNEL_NAME_PHONETIC: { - auto length = count_characters(value); - if(length > 40) { + auto length = utf8::count_characters(value); + if(length < 0 || length > 40) { /* max channel name length is 40 */ return ts::command_result{error::parameter_invalid, std::string{property::describe(property).name}}; } diff --git a/server/src/client/command_handler/client.cpp b/server/src/client/command_handler/client.cpp index b287587..a3ad850 100644 --- a/server/src/client/command_handler/client.cpp +++ b/server/src/client/command_handler/client.cpp @@ -26,6 +26,7 @@ #include #include #include +#include using namespace std::chrono; using namespace std; @@ -346,16 +347,19 @@ command_result ConnectedClient::handleCommandClientPoke(Command &cmd) { if(clients.size() > 1) { auto max_clients = this->calculate_permission(permission::i_client_poke_max_clients, 0); - if(!permission::v2::permission_granted(clients.size(), max_clients)) + if(!permission::v2::permission_granted(clients.size(), max_clients)) { return command_result{permission::i_client_poke_max_clients}; + } } auto message = cmd["msg"].string(); - if(count_characters(message) > ts::config::server::limits::poke_message_length) + if(utf8::count_characters(message) > ts::config::server::limits::poke_message_length) { return command_result{error::parameter_invalid_size, "msg"}; + } - for(auto& client : clients) + for(auto& client : clients) { client->notifyClientPoke(this->ref(), message); + } return command_result{std::forward(result)}; } @@ -535,7 +539,10 @@ command_result ConnectedClient::handleCommandClientEdit(Command &cmd, const std: } string value = cmd["client_description"].string(); - if (count_characters(value) > 200) return command_result{error::parameter_invalid, "Invalid description length. A maximum of 200 characters is allowed!"}; + auto value_length = utf8::count_characters(value); + if (value_length < 0 || value_length > 200) { + return command_result{error::parameter_invalid, "Invalid description length. A maximum of 200 characters is allowed!"}; + } } else if (info == property::CLIENT_IS_TALKER) { ACTION_REQUIRES_PERMISSION(permission::b_client_set_flag_talker, 1, client->getChannelId()); cmd["client_is_talker"] = cmd["client_is_talker"].as(); @@ -554,8 +561,13 @@ command_result ConnectedClient::handleCommandClientEdit(Command &cmd, const std: } string name = cmd["client_nickname"].string(); - if (count_characters(name) < 3) return command_result{error::parameter_invalid, "Invalid name length. A minimum of 3 characters is required!"}; - if (count_characters(name) > 30) return command_result{error::parameter_invalid, "Invalid name length. A maximum of 30 characters is allowed!"}; + auto name_length = utf8::count_characters(name); + if (name_length < 3) { + return command_result{error::parameter_invalid, "Invalid name length. A minimum of 3 characters is required!"}; + } + if (name_length > 30) { + return command_result{error::parameter_invalid, "Invalid name length. A maximum of 30 characters is allowed!"}; + } if (!permission::v2::permission_granted(1, this->calculate_permission(permission::b_client_ignore_bans, client->getClientId()))) { auto banRecord = serverInstance->banManager()->findBanByName(this->getServerId(), name); @@ -587,7 +599,10 @@ command_result ConnectedClient::handleCommandClientEdit(Command &cmd, const std: } string name = cmd["client_nickname_phonetic"].string(); - if (count_characters(name) > 30) return command_result{error::parameter_invalid, "Invalid name length. A maximum of 30 characters is allowed!"}; + auto name_length = utf8::count_characters(name); + if (name_length < 0 || name_length > 30) { + return command_result{error::parameter_invalid, "Invalid name length. A maximum of 30 characters is allowed!"}; + } } else if(info == property::CLIENT_PLAYER_VOLUME) { if(client->getType() != ClientType::CLIENT_MUSIC) return command_result{error::client_invalid_type}; if(client->properties()[property::CLIENT_OWNER] != this->getClientDatabaseId()) { diff --git a/server/src/groups/GroupManager.cpp b/server/src/groups/GroupManager.cpp index 2062fbf..0266524 100644 --- a/server/src/groups/GroupManager.cpp +++ b/server/src/groups/GroupManager.cpp @@ -2,10 +2,12 @@ // Created by WolverinDEV on 03/03/2020. // #include +#include #include #include "./GroupManager.h" #include "../InstanceHandler.h" +constexpr static auto kGroupMaxNameLength{40}; using namespace ts::server::groups; GroupManager::GroupManager(sql::SqlManager *sql, ServerId server_id, std::shared_ptr parent) @@ -161,9 +163,10 @@ void AbstractGroupManager::reset_groups(std::map &mapping) { case GroupCopyResult::UNKNOWN_SOURCE_GROUP: case GroupCopyResult::DATABASE_ERROR: logCritical(this->server_id(), "Failed to copy template group {}: {}", group->group_id(), (uint8_t) result); - break; + continue; } + assert(created_group); logTrace(this->server_id(), "Copied template group {}. New id: {}", group->group_id(), created_group->group_id()); mapping[group->group_id()] = created_group->group_id(); } @@ -269,9 +272,10 @@ std::shared_ptr AbstractGroupManager::find_group_by_name_(GroupCalculateM } GroupCreateResult AbstractGroupManager::create_group_(GroupType type, const std::string &name, std::shared_ptr& result) { - if(name.empty()) { + auto name_length = utf8::count_characters(name); + if(name_length <= 0) { return GroupCreateResult::NAME_TOO_SHORT; - } else if(name.length() > 30) { + } else if(name_length > kGroupMaxNameLength) { return GroupCreateResult::NAME_TOO_LONG; } @@ -377,7 +381,8 @@ GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, Gr GroupRenameResult AbstractGroupManager::rename_group_(GroupId group_id, const std::string &name) { std::lock_guard manage_lock{this->group_manage_mutex_}; - if(name.empty() || name.length() > 40) { + auto name_length = utf8::count_characters(name); + if(name_length <= 0 || name_length > kGroupMaxNameLength) { return GroupRenameResult::NAME_INVALID; }