-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix: assign players to a team immediately to stop malicious users #1639
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
base: master
Are you sure you want to change the base?
Conversation
...would this behavior not make more sense? I think that adding a ConVar for toggling the kicking of unassigned players would be much preferable to forcing users onto a team. |
|
allowing kicking unassigned players doesn't work in a casual or team based environment due to kicking being team based... I'm not sure how you'd hope to fix that issue to be honest in MvM it makes sense since players are always on one team... but in casual, they appear to be on a team on the client-side (in scoreboard), but are unkickable edit: also... to be honest a convar seems less likely to get accepted in this case if valve has to change anything for their servers to allow for this |
One potential solution would be allowing unassigned players to show up in the votekick menu, via a ConVar.
They're even appearing to get assigned to a team, too! |
|
what are you even proposing? A convar does not fix this issue as valve is extremely unlikely to implement that into their config... and your entire second point doesn't make sense? Players are SUPPOSED to be assigned to a team (or you'd think, since the client shows them on one team or another), and votekicking is very much intentionally team based. If you could kick unassigned players it could 100% be abused by further malicious actors to votekick players on the opposite team.. not really sure how that will fix much of anything |
Are players not actually being automatically assigned to a team here? Casual is unique in that it shows players in the team list as soon as they start a connection to the server. I'm saying that being able to kick players at that stage would probably make more sense. That wouldn't involve players from the other team being able to kick them. |
|
I mean that's the entire issue, they're not properly assigned to a team.. it's not so much the unconnected as much as it is they're not on a team. |
|
I guess I'm just confused on what's actually dictating what team they're showing up on before they actually get 'assigned' to one, then. |
|
I'm not sure, i'll look into it later; i'm much much too tired at the moment to take a look. I'm not sure what else could be done to fix it though, as I said- I don't think a convar is a very good option here (both in terms of getting accepted, and the actual purpose of it) |
Just being able to kick players in that limbo state. Since it seems to be Casual-exclusive behavior, a ConVar probably wouldn't be necessary, yeah. |
|
Thank you for making this and valve really should merge this soon |
|
Alright i looked into it more and... the scoreboard shows players on their correct teams because the scoreboard uses the state from the matchmaking system. Kicking however uses the state of the actual entities and players aren't on a team until after they click continue on the motd/intros. An alternative fix would be to make kicking use the matchmaking state instead if available but i'm not sure how that would behave with custom gamemodes which move players to different teams constantly, eg: Zombie Infection. Being able to kick players who are not assigned to a team causes the issue that you can essentially kick players on the other team while they are on the motd/intro screen, just because they are unassigned doesn't mean they are on your team or that they are bots, players go afk while queuing for example. That fix worked for MVM because there is only one human team but in regular modes there are two human teams so this doesn't really work. |
|
havent looked into how this exploit works, but why not just kick players for being afk if they've been unassigned for like 2 mins |
|
I mean if they're obviously bots it's probably for the better to be able to kick them as soon as you can |
|
@wgetJane kicking bots when afk is already done (5 minutes), and also pretty easily bypassable by bad actors by doing as something as simple as sending a movement packet (however the afk bots seem to requeue before those 5 minutes are reached at the current moment) Another issue is that valve kind of has an obligation to account for people with slow pcs, so anything that's done has to do that. |
|
I think adding a separate unassigned timer would fix this, if you stay in unassigned for too long, you get kicked. You could make it a static amount like 60 seconds, or like half of the idle time. Also another thing would be to fix the GC team assignments, since this seems to be mostly abused in Casual. |
|
@mastercoms That could fix it yeah but this also seems like unintended behavior in general. There was a bug fix in 2018 that was supposed to fix exactly this but it just does not work for whatever reason. Another issue is map intros, for example the VSH map intro is 37 seconds (although this is quite a small issue, it could be relevant) I'm confused what you mean by your second point tho... that's exactly what this PR fixes? The GC already assigns players to a team but the server is waiting for the MOTD to be passed for no reason. If you mean making the kick system respect the GC instead of the server... that would require MAJOR reworks and is also quite hard to test as a PR for anyone other than valve (unless you want to write your own custom GC... which arguably is breaking some agreements with valve iirc?) |
|
Youre not taking into consideration that sometimes people join just to spectate and leave, like admins checking players. So I think they should be moved to spectators if mp_allowspectators is set to 1 and mp_forceautoteam is set to 0. Also does your change take into account Replay and SourceTV ? These need to spectate as they are not real players. |
|
This change only applies to force autoteam and gets their autoteam |
The code works fine with bots (Including SourceTV/Relay), I tested it. Please do not leave comments unless you actually understand the code here, that's all i really have to say as mastercoms essentially explained it perfectly |
|
FYI you can kick these players through the console, you don't really have to force them onto teams, just include them in the votekick even if they don't have player entities yet. Nevermind, seems they patched this out, I wonder why |
|
They probably patched it out because people would try to kick others on the opposite team |
|
Seems like they only check if the player you're kicking is on your team if you're running tf2 in debug mode for some reason
Seems like in that case the best solution is indeed to force the players to choose a team. |
Description
Assigns players to a team immediately after joining the server, before the server would only assign players to teams after viewing the MOTD and intros, malicious players would abuse this to never be assigned to a team and they couldn't be kicked because the kick system ignores players who aren't assigned to a team.
MvM already specifically has this fixed by simply allowing kicking unassigned players, this PR properly fixes it for all modes.
This was tested with and without
mp_forceautoteamonitemtest,tc_hydro,zi_blazehattan, andmvm_coaltown