From bf7212889bb2c189b215bd4b78b596b951a50590 Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 11 May 2023 23:56:40 +0300 Subject: [PATCH 1/7] Fix invalid client index in Basebans menu Fix #1768 The `sm_admin`-triggered Menu flow for banning players is caching client indices inside the basebans.sp `PlayerInfo` struct, and can then incorrectly use them in the menu callback without checking for the related client's UserId validity. This leads to bug #1768 occurring when the ban target player disconnects from the server before the banning admin could complete the banmenu UI flow. Since the related menu callbacks can't rely on the cached client index, I have removed the basebans.sp `PlayerInfo.banTarget` member entirely, in favor of the `PlayerInfo.banTarget`, and instead call `GetClientOfUserId(...)` to get & validate the target client index where necessary. The `PrepareBan(...)` function has been refactored to take a ban target UserId (instead of the target client index) accordingly. --- plugins/basebans.sp | 3 +- plugins/basebans/ban.sp | 75 +++++++++++++++++++++++++---------------- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/plugins/basebans.sp b/plugins/basebans.sp index 986afd17fb..3e6879711d 100644 --- a/plugins/basebans.sp +++ b/plugins/basebans.sp @@ -51,7 +51,6 @@ public Plugin myinfo = TopMenu hTopMenu; enum struct PlayerInfo { - int banTarget; int banTargetUserId; int banTime; int isWaitingForChatReason; @@ -387,7 +386,7 @@ public Action OnClientSayCommand(int client, const char[] command, const char[] if(playerinfo[client].isWaitingForChatReason) { playerinfo[client].isWaitingForChatReason = false; - PrepareBan(client, playerinfo[client].banTarget, playerinfo[client].banTime, sArgs); + PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTime, sArgs); return Plugin_Stop; } diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index 813769f885..f4126e135b 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -31,21 +31,18 @@ * Version: $Id$ */ -void PrepareBan(int client, int target, int time, const char[] reason) +void PrepareBan(int adminClient, int banTargetUserId, int time, const char[] reason) { - int originalTarget = GetClientOfUserId(playerinfo[client].banTargetUserId); - - if (originalTarget != target) + if (adminClient == 0) { - if (client == 0) - { - PrintToServer("[SM] %t", "Player no longer available"); - } - else - { - PrintToChat(client, "[SM] %t", "Player no longer available"); - } + PrintToServer("[SM] %t", "Player no longer available"); + return; + } + int target = GetClientOfUserId(banTargetUserId); + if (target == 0) + { + PrintToChat(adminClient, "[SM] %t", "Player no longer available"); return; } @@ -56,34 +53,34 @@ void PrepareBan(int client, int target, int time, const char[] reason) { if (reason[0] == '\0') { - ShowActivity(client, "%t", "Permabanned player", name); + ShowActivity(adminClient, "%t", "Permabanned player", name); } else { - ShowActivity(client, "%t", "Permabanned player reason", name, reason); + ShowActivity(adminClient, "%t", "Permabanned player reason", name, reason); } } else { if (reason[0] == '\0') { - ShowActivity(client, "%t", "Banned player", name, time); + ShowActivity(adminClient, "%t", "Banned player", name, time); } else { - ShowActivity(client, "%t", "Banned player reason", name, time, reason); + ShowActivity(adminClient, "%t", "Banned player reason", name, time, reason); } } - LogAction(client, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", client, target, time, reason); + LogAction(adminClient, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", adminClient, target, time, reason); if (reason[0] == '\0') { - BanClient(target, time, BANFLAG_AUTO, "Banned", "Banned", "sm_ban", client); + BanClient(target, time, BANFLAG_AUTO, "Banned", "Banned", "sm_ban", adminClient); } else { - BanClient(target, time, BANFLAG_AUTO, reason, reason, "sm_ban", client); + BanClient(target, time, BANFLAG_AUTO, reason, reason, "sm_ban", adminClient); } } @@ -103,10 +100,17 @@ void DisplayBanTargetMenu(int client) void DisplayBanTimeMenu(int client) { + int target = GetClientOfUserId(playerinfo[client].banTargetUserId); + if (target == 0) + { + PrintToChat(client, "[SM] %t", "Player no longer available"); + return; + } + Menu menu = new Menu(MenuHandler_BanTimeList); char title[100]; - Format(title, sizeof(title), "%T: %N", "Ban player", client, playerinfo[client].banTarget); + Format(title, sizeof(title), "%T: %N", "Ban player", client, target); menu.SetTitle(title); menu.ExitBackButton = true; @@ -123,10 +127,17 @@ void DisplayBanTimeMenu(int client) void DisplayBanReasonMenu(int client) { + int target = GetClientOfUserId(playerinfo[client].banTargetUserId); + if (target == 0) + { + PrintToChat(client, "[SM] %t", "Player no longer available"); + return; + } + Menu menu = new Menu(MenuHandler_BanReasonList); char title[100]; - Format(title, sizeof(title), "%T: %N", "Ban reason", client, playerinfo[client].banTarget); + Format(title, sizeof(title), "%T: %N", "Ban reason", client, target); menu.SetTitle(title); menu.ExitBackButton = true; @@ -201,8 +212,8 @@ public int MenuHandler_BanReasonList(Menu menu, MenuAction action, int param1, i char info[64]; menu.GetItem(param2, info, sizeof(info)); - - PrepareBan(param1, playerinfo[param1].banTarget, playerinfo[param1].banTime, info); + + PrepareBan(param1, playerinfo[param1].banTargetUserId, playerinfo[param1].banTime, info); } } @@ -240,7 +251,6 @@ public int MenuHandler_BanPlayerList(Menu menu, MenuAction action, int param1, i } else { - playerinfo[param1].banTarget = target; playerinfo[param1].banTargetUserId = userid; DisplayBanTimeMenu(param1); } @@ -264,12 +274,19 @@ public int MenuHandler_BanTimeList(Menu menu, MenuAction action, int param1, int } else if (action == MenuAction_Select) { - char info[32]; + if (GetClientOfUserId(playerinfo[param1].banTargetUserId) == 0) + { + PrintToChat(param1, "[SM] %t", "Player no longer available"); + } + else + { + char info[32]; - menu.GetItem(param2, info, sizeof(info)); - playerinfo[param1].banTime = StringToInt(info); + menu.GetItem(param2, info, sizeof(info)); + playerinfo[param1].banTime = StringToInt(info); - DisplayBanReasonMenu(param1); + DisplayBanReasonMenu(param1); + } } return 0; @@ -318,7 +335,7 @@ public Action Command_Ban(int client, int args) playerinfo[client].banTargetUserId = GetClientUserId(target); int time = StringToInt(s_time); - PrepareBan(client, target, time, Arguments[len]); + PrepareBan(client, playerinfo[client].banTargetUserId, time, Arguments[len]); return Plugin_Handled; } From f9ec38103a2624cfcc240f3d82805ef18a51a398 Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 15 Oct 2023 19:56:51 +0300 Subject: [PATCH 2/7] Use auth strings when banning from menu When an admin initiates a ban from a BaseBans GUI menu, track players by their AuthId (SteamID) instead of userid, so that the admin is able to target players who disconnect after that menu was constructed. If the target player is still connected, use their userid to ban the same as before this commit. If the target has disconnected, write a ban using their stored AuthId value, as with "sm_addban". This commit also revertss the `(admin==0)` check from PrepareBan that was mistakenly added by bf7212889bb2c189b215bd4b78b596b951a50590, because it would prevent banning from server/RCON (client index 0). --- plugins/basebans.sp | 3 +- plugins/basebans/ban.sp | 220 ++++++++++++++++++++++++++++------------ 2 files changed, 155 insertions(+), 68 deletions(-) diff --git a/plugins/basebans.sp b/plugins/basebans.sp index 3e6879711d..d7ea83e1fa 100644 --- a/plugins/basebans.sp +++ b/plugins/basebans.sp @@ -54,6 +54,7 @@ enum struct PlayerInfo { int banTargetUserId; int banTime; int isWaitingForChatReason; + char banTargetAuthId[MAX_AUTHID_LENGTH]; } PlayerInfo playerinfo[MAXPLAYERS+1]; @@ -386,7 +387,7 @@ public Action OnClientSayCommand(int client, const char[] command, const char[] if(playerinfo[client].isWaitingForChatReason) { playerinfo[client].isWaitingForChatReason = false; - PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTime, sArgs); + PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, playerinfo[client].banTime, sArgs); return Plugin_Stop; } diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index f4126e135b..8606da0f7d 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -31,23 +31,18 @@ * Version: $Id$ */ -void PrepareBan(int adminClient, int banTargetUserId, int time, const char[] reason) +// Because GetTargetName will return either client name, or client authid, +// and we need to be able to fit whichever is larger to a buffer. +#if MAX_AUTHID_LENGTH > MAX_NAME_LENGTH +#define MAX_TARGETNAME_LENGTH MAX_AUTHID_LENGTH +#else +#define MAX_TARGETNAME_LENGTH MAX_NAME_LENGTH +#endif + +void PrepareBan(int adminClient, int banTargetUserId, const char[] banTargetAuthId, int time, const char[] reason) { - if (adminClient == 0) - { - PrintToServer("[SM] %t", "Player no longer available"); - return; - } - - int target = GetClientOfUserId(banTargetUserId); - if (target == 0) - { - PrintToChat(adminClient, "[SM] %t", "Player no longer available"); - return; - } - - char name[MAX_NAME_LENGTH]; - GetClientName(target, name, sizeof(name)); + char name[MAX_TARGETNAME_LENGTH]; + GetTargetName(banTargetUserId, banTargetAuthId, name, sizeof(name)); if (!time) { @@ -72,15 +67,30 @@ void PrepareBan(int adminClient, int banTargetUserId, int time, const char[] rea } } - LogAction(adminClient, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", adminClient, target, time, reason); - - if (reason[0] == '\0') + int target = GetClientOfAuthId(banTargetAuthId); + // Ban & kick if target is connected, else record the ban with authid + if (target != 0) { - BanClient(target, time, BANFLAG_AUTO, "Banned", "Banned", "sm_ban", adminClient); + LogAction(adminClient, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", adminClient, target, time, reason); + + BanClient(target, time, BANFLAG_AUTO, + (reason[0] == '\0') ? "Banned" : reason, + (reason[0] == '\0') ? "Banned" : reason, + "sm_ban", adminClient); } else { - BanClient(target, time, BANFLAG_AUTO, reason, reason, "sm_ban", adminClient); + LogAction(adminClient, + target, + "\"%L\" added ban (minutes \"%d\") (id \"%s\") (reason \"%s\")", + adminClient, + time, + banTargetAuthId, + reason); + + BanIdentity(banTargetAuthId, time, BANFLAG_AUTHID, + (reason[0] == '\0') ? "Banned" : reason, + "sm_addban", adminClient); } } @@ -93,24 +103,20 @@ void DisplayBanTargetMenu(int client) menu.SetTitle(title); menu.ExitBackButton = CheckCommandAccess(client, "sm_admin", ADMFLAG_GENERIC, false); - AddTargetsToMenu2(menu, client, COMMAND_FILTER_NO_BOTS|COMMAND_FILTER_CONNECTED); + AddTargetsToMenuByAuthId(menu, client, COMMAND_FILTER_NO_BOTS|COMMAND_FILTER_CONNECTED, AuthId_Steam2); menu.Display(client, MENU_TIME_FOREVER); } void DisplayBanTimeMenu(int client) { - int target = GetClientOfUserId(playerinfo[client].banTargetUserId); - if (target == 0) - { - PrintToChat(client, "[SM] %t", "Player no longer available"); - return; - } + char targetName[MAX_TARGETNAME_LENGTH]; + GetTargetName(playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, targetName, sizeof(targetName)); Menu menu = new Menu(MenuHandler_BanTimeList); char title[100]; - Format(title, sizeof(title), "%T: %N", "Ban player", client, target); + Format(title, sizeof(title), "%T: %s", "Ban player", client, targetName); menu.SetTitle(title); menu.ExitBackButton = true; @@ -127,17 +133,13 @@ void DisplayBanTimeMenu(int client) void DisplayBanReasonMenu(int client) { - int target = GetClientOfUserId(playerinfo[client].banTargetUserId); - if (target == 0) - { - PrintToChat(client, "[SM] %t", "Player no longer available"); - return; - } + char name[MAX_TARGETNAME_LENGTH]; + GetTargetName(playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, name, sizeof(name)); Menu menu = new Menu(MenuHandler_BanReasonList); char title[100]; - Format(title, sizeof(title), "%T: %N", "Ban reason", client, target); + Format(title, sizeof(title), "%T: %s", "Ban reason", client, name); menu.SetTitle(title); menu.ExitBackButton = true; @@ -210,10 +212,10 @@ public int MenuHandler_BanReasonList(Menu menu, MenuAction action, int param1, i else { char info[64]; - + menu.GetItem(param2, info, sizeof(info)); - PrepareBan(param1, playerinfo[param1].banTargetUserId, playerinfo[param1].banTime, info); + PrepareBan(param1, playerinfo[param1].banTargetUserId, playerinfo[param1].banTargetAuthId, playerinfo[param1].banTime, info); } } @@ -235,25 +237,13 @@ public int MenuHandler_BanPlayerList(Menu menu, MenuAction action, int param1, i } else if (action == MenuAction_Select) { - char info[32], name[32]; - int userid, target; + char name[32]; + menu.GetItem(param2, playerinfo[param1].banTargetAuthId, sizeof(PlayerInfo::banTargetAuthId), _, name, sizeof(name)); - menu.GetItem(param2, info, sizeof(info), _, name, sizeof(name)); - userid = StringToInt(info); + int target = GetClientOfAuthId(playerinfo[param1].banTargetAuthId); + playerinfo[param1].banTargetUserId = (target == 0) ? 0 : GetClientUserId(target); - if ((target = GetClientOfUserId(userid)) == 0) - { - PrintToChat(param1, "[SM] %t", "Player no longer available"); - } - else if (!CanUserTarget(param1, target)) - { - PrintToChat(param1, "[SM] %t", "Unable to target"); - } - else - { - playerinfo[param1].banTargetUserId = userid; - DisplayBanTimeMenu(param1); - } + DisplayBanTimeMenu(param1); } return 0; @@ -274,19 +264,12 @@ public int MenuHandler_BanTimeList(Menu menu, MenuAction action, int param1, int } else if (action == MenuAction_Select) { - if (GetClientOfUserId(playerinfo[param1].banTargetUserId) == 0) - { - PrintToChat(param1, "[SM] %t", "Player no longer available"); - } - else - { - char info[32]; + char info[32]; - menu.GetItem(param2, info, sizeof(info)); - playerinfo[param1].banTime = StringToInt(info); + menu.GetItem(param2, info, sizeof(info)); + playerinfo[param1].banTime = StringToInt(info); - DisplayBanReasonMenu(param1); - } + DisplayBanReasonMenu(param1); } return 0; @@ -333,9 +316,112 @@ public Action Command_Ban(int client, int args) } playerinfo[client].banTargetUserId = GetClientUserId(target); + GetClientAuthId(target, AuthId_Steam2, playerinfo[client].banTargetAuthId, sizeof(PlayerInfo::banTargetAuthId)); int time = StringToInt(s_time); - PrepareBan(client, playerinfo[client].banTargetUserId, time, Arguments[len]); + PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, time, Arguments[len]); return Plugin_Handled; } + +int GetClientOfAuthId(const char[] authid, AuthIdType authIdType=AuthId_Steam2) +{ + char compareId[MAX_AUTHID_LENGTH]; + for (int client = 1; client <= MaxClients; client++) + { + if (!IsClientInGame(client) || !IsClientAuthorized(client)) + { + continue; + } + if (!GetClientAuthId(client, authIdType, compareId, sizeof(compareId))) + { + continue; + } + if (StrEqual(compareId, authid)) + { + return client; + } + } + + return 0; +} + +void GetTargetName(int userid, const char[] authid, char[] out, int maxlen) +{ + int client = GetClientOfUserId(userid); + if (client) + { + Format(out, maxlen, "%N", client); + } + else + { + // if client was not in game, use their authid as the name instead + strcopy(out, maxlen, authid); + } +} + +/** + * Adds targets to an admin menu. + * + * Each client is displayed as: name (authid) + * Each item contains the authid as a string for its info. + * + * @param menu Menu Handle. + * @param source_client Source client, or 0 to ignore immunity. + * @param flags COMMAND_FILTER flags from commandfilters.inc. + * @return Number of clients added. + */ +int AddTargetsToMenuByAuthId(Menu menu, int source_client, int flags, AuthIdType authIdType) +{ + char authid[MAX_AUTHID_LENGTH]; + char name[MAX_NAME_LENGTH]; + char display[sizeof(name) + sizeof(authid) + 3]; + + int num_clients; + + for (int i = 1; i <= MaxClients; i++) + { + if (!IsClientConnected(i) || IsClientInKickQueue(i)) + { + continue; + } + + if (((flags & COMMAND_FILTER_NO_BOTS) == COMMAND_FILTER_NO_BOTS) + && IsFakeClient(i)) + { + continue; + } + + if (((flags & COMMAND_FILTER_CONNECTED) != COMMAND_FILTER_CONNECTED) + && !IsClientInGame(i)) + { + continue; + } + + if (((flags & COMMAND_FILTER_ALIVE) == COMMAND_FILTER_ALIVE) + && !IsPlayerAlive(i)) + { + continue; + } + + if (((flags & COMMAND_FILTER_DEAD) == COMMAND_FILTER_DEAD) + && IsPlayerAlive(i)) + { + continue; + } + + if ((source_client && ((flags & COMMAND_FILTER_NO_IMMUNITY) != COMMAND_FILTER_NO_IMMUNITY)) + && !CanUserTarget(source_client, i)) + { + continue; + } + + GetClientAuthId(i, authIdType, authid, sizeof(authid)); + GetClientName(i, name, sizeof(name)); + Format(display, sizeof(display), "%s (%s)", name, authid); + menu.AddItem(authid, display); + num_clients++; + } + + return num_clients; +} \ No newline at end of file From 5ac6be049781d297d9a15198528b79d644af1f64 Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 15 Oct 2023 20:49:37 +0300 Subject: [PATCH 3/7] Use userid to find client when possible Only fall back to authid lookup if userid no longer resolved to client index --- plugins/basebans/ban.sp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index 8606da0f7d..9a53d04758 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -67,7 +67,12 @@ void PrepareBan(int adminClient, int banTargetUserId, const char[] banTargetAuth } } - int target = GetClientOfAuthId(banTargetAuthId); + int target = (banTargetUserId == 0) ? 0 : GetClientOfUserId(banTargetUserId); + if (target == 0) + { + target = GetClientOfAuthId(banTargetAuthId); + } + // Ban & kick if target is connected, else record the ban with authid if (target != 0) { From 67f5fcdb25a1c7cd361d8cd228ba447687489ff0 Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 15 Oct 2023 20:58:56 +0300 Subject: [PATCH 4/7] Refactor In function "PrepareBan", use the global PlayerInfo struct array members directly, instead of passing them as function parameters. --- plugins/basebans.sp | 2 +- plugins/basebans/ban.sp | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/plugins/basebans.sp b/plugins/basebans.sp index d7ea83e1fa..3809c53da9 100644 --- a/plugins/basebans.sp +++ b/plugins/basebans.sp @@ -387,7 +387,7 @@ public Action OnClientSayCommand(int client, const char[] command, const char[] if(playerinfo[client].isWaitingForChatReason) { playerinfo[client].isWaitingForChatReason = false; - PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, playerinfo[client].banTime, sArgs); + PrepareBan(client, playerinfo[client].banTime, sArgs); return Plugin_Stop; } diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index 9a53d04758..9afa7efcb0 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -39,10 +39,10 @@ #define MAX_TARGETNAME_LENGTH MAX_NAME_LENGTH #endif -void PrepareBan(int adminClient, int banTargetUserId, const char[] banTargetAuthId, int time, const char[] reason) +void PrepareBan(int adminClient, int time, const char[] reason) { char name[MAX_TARGETNAME_LENGTH]; - GetTargetName(banTargetUserId, banTargetAuthId, name, sizeof(name)); + GetTargetName(playerinfo[adminClient].banTargetUserId, playerinfo[adminClient].banTargetAuthId, name, sizeof(name)); if (!time) { @@ -67,16 +67,24 @@ void PrepareBan(int adminClient, int banTargetUserId, const char[] banTargetAuth } } - int target = (banTargetUserId == 0) ? 0 : GetClientOfUserId(banTargetUserId); + int target = (playerinfo[adminClient].banTargetUserId == 0) + ? 0 : GetClientOfUserId(playerinfo[adminClient].banTargetUserId); + if (target == 0) { - target = GetClientOfAuthId(banTargetAuthId); + target = GetClientOfAuthId(playerinfo[adminClient].banTargetAuthId); } // Ban & kick if target is connected, else record the ban with authid if (target != 0) { - LogAction(adminClient, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", adminClient, target, time, reason); + LogAction(adminClient, + target, + "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", + adminClient, + target, + time, + reason); BanClient(target, time, BANFLAG_AUTO, (reason[0] == '\0') ? "Banned" : reason, @@ -90,10 +98,10 @@ void PrepareBan(int adminClient, int banTargetUserId, const char[] banTargetAuth "\"%L\" added ban (minutes \"%d\") (id \"%s\") (reason \"%s\")", adminClient, time, - banTargetAuthId, + playerinfo[adminClient].banTargetAuthId, reason); - BanIdentity(banTargetAuthId, time, BANFLAG_AUTHID, + BanIdentity(playerinfo[adminClient].banTargetAuthId, time, BANFLAG_AUTHID, (reason[0] == '\0') ? "Banned" : reason, "sm_addban", adminClient); } @@ -220,7 +228,7 @@ public int MenuHandler_BanReasonList(Menu menu, MenuAction action, int param1, i menu.GetItem(param2, info, sizeof(info)); - PrepareBan(param1, playerinfo[param1].banTargetUserId, playerinfo[param1].banTargetAuthId, playerinfo[param1].banTime, info); + PrepareBan(param1, playerinfo[param1].banTime, info); } } @@ -324,7 +332,7 @@ public Action Command_Ban(int client, int args) GetClientAuthId(target, AuthId_Steam2, playerinfo[client].banTargetAuthId, sizeof(PlayerInfo::banTargetAuthId)); int time = StringToInt(s_time); - PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, time, Arguments[len]); + PrepareBan(client, time, Arguments[len]); return Plugin_Handled; } From 9b06d85771c2245ac7f0817fc40d48efe6e2e488 Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 15 Oct 2023 21:29:51 +0300 Subject: [PATCH 5/7] Don't attempt to get auth string for unauthorized clients --- plugins/basebans/ban.sp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index 9afa7efcb0..61109f975e 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -399,6 +399,11 @@ int AddTargetsToMenuByAuthId(Menu menu, int source_client, int flags, AuthIdType continue; } + if (!IsClientAuthorized(i)) + { + continue; + } + if (((flags & COMMAND_FILTER_NO_BOTS) == COMMAND_FILTER_NO_BOTS) && IsFakeClient(i)) { From 3b6f18e3b4a24e14f6159e449a35aad9e630c65c Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 15 Oct 2023 22:05:25 +0300 Subject: [PATCH 6/7] Fix client validity checks Unify the client validity checks with AddTargetsToMenuByAuthId checks --- plugins/basebans/ban.sp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index 61109f975e..a00987194e 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -342,7 +342,7 @@ int GetClientOfAuthId(const char[] authid, AuthIdType authIdType=AuthId_Steam2) char compareId[MAX_AUTHID_LENGTH]; for (int client = 1; client <= MaxClients; client++) { - if (!IsClientInGame(client) || !IsClientAuthorized(client)) + if (!IsClientConnected(client) || IsClientInKickQueue(client) || !IsClientAuthorized(client)) { continue; } From 69988c8b98f70bf840de73fe1f89561dff9099d0 Mon Sep 17 00:00:00 2001 From: Rain Date: Sat, 9 Dec 2023 11:21:57 +0200 Subject: [PATCH 7/7] Use MAX_TARGET_LENGTH Use MAX_TARGET_LENGTH over defining a new custom length --- plugins/basebans/ban.sp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/plugins/basebans/ban.sp b/plugins/basebans/ban.sp index a00987194e..d6bc3bbe47 100644 --- a/plugins/basebans/ban.sp +++ b/plugins/basebans/ban.sp @@ -31,17 +31,9 @@ * Version: $Id$ */ -// Because GetTargetName will return either client name, or client authid, -// and we need to be able to fit whichever is larger to a buffer. -#if MAX_AUTHID_LENGTH > MAX_NAME_LENGTH -#define MAX_TARGETNAME_LENGTH MAX_AUTHID_LENGTH -#else -#define MAX_TARGETNAME_LENGTH MAX_NAME_LENGTH -#endif - void PrepareBan(int adminClient, int time, const char[] reason) { - char name[MAX_TARGETNAME_LENGTH]; + char name[MAX_TARGET_LENGTH]; GetTargetName(playerinfo[adminClient].banTargetUserId, playerinfo[adminClient].banTargetAuthId, name, sizeof(name)); if (!time) @@ -123,7 +115,7 @@ void DisplayBanTargetMenu(int client) void DisplayBanTimeMenu(int client) { - char targetName[MAX_TARGETNAME_LENGTH]; + char targetName[MAX_TARGET_LENGTH]; GetTargetName(playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, targetName, sizeof(targetName)); Menu menu = new Menu(MenuHandler_BanTimeList); @@ -146,7 +138,7 @@ void DisplayBanTimeMenu(int client) void DisplayBanReasonMenu(int client) { - char name[MAX_TARGETNAME_LENGTH]; + char name[MAX_TARGET_LENGTH]; GetTargetName(playerinfo[client].banTargetUserId, playerinfo[client].banTargetAuthId, name, sizeof(name)); Menu menu = new Menu(MenuHandler_BanReasonList);