-
-
Notifications
You must be signed in to change notification settings - Fork 432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Basebans client index is invalid #1768
Comments
Rainyan
added a commit
to Rainyan/sourcemod
that referenced
this issue
May 11, 2023
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.
Rainyan
added a commit
to Rainyan/sourcemod
that referenced
this issue
May 11, 2023
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 banmenu 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 call `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.
Rainyan
added a commit
to Rainyan/sourcemod
that referenced
this issue
May 11, 2023
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 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Help us help you
Environment
Description
Not sure how to reproduce.
Problematic Code (or Steps to Reproduce)
sourcemod/plugins/basebans/ban.sp
Line 129 in 3bafc1e
Logs
The text was updated successfully, but these errors were encountered: