Skip to content

Commit 0ebdc72

Browse files
RainyanRainyan
committed
Fix invalid client index in Basebans menu
Fix alliedmodders#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 alliedmodders#1768 occurring when the ban target player disconnects from the server before the banning admin could complete the banning Menu UI flow. Since the related Menu callbacks can't rely on the client index, I have removed the basebans.sp `PlayerInfo.banTarget` member entirely, in favor of the `PlayerInfo.banTargetUserId`, and instead of `GetClientOfUserId(...)` to get & validate the client index where necessary. The `PrepareBan(...)` function has been refactored to take a ban target UserId (instead of the client index) accordingly.
1 parent 97f2fc9 commit 0ebdc72

File tree

2 files changed

+47
-31
lines changed

2 files changed

+47
-31
lines changed

Diff for: ‎plugins/basebans.sp

+1-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ public Plugin myinfo =
5151
TopMenu hTopMenu;
5252

5353
enum struct PlayerInfo {
54-
int banTarget;
5554
int banTargetUserId;
5655
int banTime;
5756
int isWaitingForChatReason;
@@ -387,7 +386,7 @@ public Action OnClientSayCommand(int client, const char[] command, const char[]
387386
if(playerinfo[client].isWaitingForChatReason)
388387
{
389388
playerinfo[client].isWaitingForChatReason = false;
390-
PrepareBan(client, playerinfo[client].banTarget, playerinfo[client].banTime, sArgs);
389+
PrepareBan(client, playerinfo[client].banTargetUserId, playerinfo[client].banTime, sArgs);
391390
return Plugin_Stop;
392391
}
393392

Diff for: ‎plugins/basebans/ban.sp

+46-29
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,18 @@
3131
* Version: $Id$
3232
*/
3333

34-
void PrepareBan(int client, int target, int time, const char[] reason)
34+
void PrepareBan(int adminClient, int banTargetUserId, int time, const char[] reason)
3535
{
36-
int originalTarget = GetClientOfUserId(playerinfo[client].banTargetUserId);
37-
38-
if (originalTarget != target)
36+
if (adminClient == 0)
3937
{
40-
if (client == 0)
41-
{
42-
PrintToServer("[SM] %t", "Player no longer available");
43-
}
44-
else
45-
{
46-
PrintToChat(client, "[SM] %t", "Player no longer available");
47-
}
38+
PrintToServer("[SM] %t", "Player no longer available");
39+
return;
40+
}
4841

42+
int target = GetClientOfUserId(banTargetUserId);
43+
if (target == 0)
44+
{
45+
PrintToServer("[SM] %t", "Player no longer available");
4946
return;
5047
}
5148

@@ -56,34 +53,34 @@ void PrepareBan(int client, int target, int time, const char[] reason)
5653
{
5754
if (reason[0] == '\0')
5855
{
59-
ShowActivity(client, "%t", "Permabanned player", name);
56+
ShowActivity(adminClient, "%t", "Permabanned player", name);
6057
}
6158
else
6259
{
63-
ShowActivity(client, "%t", "Permabanned player reason", name, reason);
60+
ShowActivity(adminClient, "%t", "Permabanned player reason", name, reason);
6461
}
6562
}
6663
else
6764
{
6865
if (reason[0] == '\0')
6966
{
70-
ShowActivity(client, "%t", "Banned player", name, time);
67+
ShowActivity(adminClient, "%t", "Banned player", name, time);
7168
}
7269
else
7370
{
74-
ShowActivity(client, "%t", "Banned player reason", name, time, reason);
71+
ShowActivity(adminClient, "%t", "Banned player reason", name, time, reason);
7572
}
7673
}
7774

78-
LogAction(client, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", client, target, time, reason);
75+
LogAction(adminClient, target, "\"%L\" banned \"%L\" (minutes \"%d\") (reason \"%s\")", adminClient, target, time, reason);
7976

8077
if (reason[0] == '\0')
8178
{
82-
BanClient(target, time, BANFLAG_AUTO, "Banned", "Banned", "sm_ban", client);
79+
BanClient(target, time, BANFLAG_AUTO, "Banned", "Banned", "sm_ban", adminClient);
8380
}
8481
else
8582
{
86-
BanClient(target, time, BANFLAG_AUTO, reason, reason, "sm_ban", client);
83+
BanClient(target, time, BANFLAG_AUTO, reason, reason, "sm_ban", adminClient);
8784
}
8885
}
8986

@@ -103,10 +100,17 @@ void DisplayBanTargetMenu(int client)
103100

104101
void DisplayBanTimeMenu(int client)
105102
{
103+
int target = GetClientOfUserId(playerinfo[client].banTargetUserId);
104+
if (target == 0)
105+
{
106+
PrintToChat(client, "[SM] %t", "Player no longer available");
107+
return;
108+
}
109+
106110
Menu menu = new Menu(MenuHandler_BanTimeList);
107111

108112
char title[100];
109-
Format(title, sizeof(title), "%T: %N", "Ban player", client, playerinfo[client].banTarget);
113+
Format(title, sizeof(title), "%T: %N", "Ban player", client, target);
110114
menu.SetTitle(title);
111115
menu.ExitBackButton = true;
112116

@@ -123,10 +127,17 @@ void DisplayBanTimeMenu(int client)
123127

124128
void DisplayBanReasonMenu(int client)
125129
{
130+
int target = GetClientOfUserId(playerinfo[client].banTargetUserId);
131+
if (target == 0)
132+
{
133+
PrintToChat(client, "[SM] %t", "Player no longer available");
134+
return;
135+
}
136+
126137
Menu menu = new Menu(MenuHandler_BanReasonList);
127138

128139
char title[100];
129-
Format(title, sizeof(title), "%T: %N", "Ban reason", client, playerinfo[client].banTarget);
140+
Format(title, sizeof(title), "%T: %N", "Ban reason", client, target);
130141
menu.SetTitle(title);
131142
menu.ExitBackButton = true;
132143

@@ -201,8 +212,8 @@ public int MenuHandler_BanReasonList(Menu menu, MenuAction action, int param1, i
201212
char info[64];
202213

203214
menu.GetItem(param2, info, sizeof(info));
204-
205-
PrepareBan(param1, playerinfo[param1].banTarget, playerinfo[param1].banTime, info);
215+
216+
PrepareBan(param1, playerinfo[param1].banTargetUserId, playerinfo[param1].banTime, info);
206217
}
207218
}
208219

@@ -240,7 +251,6 @@ public int MenuHandler_BanPlayerList(Menu menu, MenuAction action, int param1, i
240251
}
241252
else
242253
{
243-
playerinfo[param1].banTarget = target;
244254
playerinfo[param1].banTargetUserId = userid;
245255
DisplayBanTimeMenu(param1);
246256
}
@@ -264,12 +274,19 @@ public int MenuHandler_BanTimeList(Menu menu, MenuAction action, int param1, int
264274
}
265275
else if (action == MenuAction_Select)
266276
{
267-
char info[32];
277+
if (GetClientOfUserId(playerinfo[param1].banTargetUserId) == 0)
278+
{
279+
PrintToChat(param1, "[SM] %t", "Player no longer available");
280+
}
281+
else
282+
{
283+
char info[32];
268284

269-
menu.GetItem(param2, info, sizeof(info));
270-
playerinfo[param1].banTime = StringToInt(info);
285+
menu.GetItem(param2, info, sizeof(info));
286+
playerinfo[param1].banTime = StringToInt(info);
271287

272-
DisplayBanReasonMenu(param1);
288+
DisplayBanReasonMenu(param1);
289+
}
273290
}
274291

275292
return 0;
@@ -318,7 +335,7 @@ public Action Command_Ban(int client, int args)
318335
playerinfo[client].banTargetUserId = GetClientUserId(target);
319336

320337
int time = StringToInt(s_time);
321-
PrepareBan(client, target, time, Arguments[len]);
338+
PrepareBan(client, playerinfo[client].banTargetUserId, time, Arguments[len]);
322339

323340
return Plugin_Handled;
324341
}

0 commit comments

Comments
 (0)