Votes improperly pass on map change

[sneak-it]

Describe the bug
When someone creates a change map vote that is set to end outside the bounds of the current map timelimit (map will change before it is over), the vote will always pass.

To Reproduce
Steps to reproduce the behavior:
Config:

	"Changelevel"
	{
		"type"			"map"
		"vote"			"1"
		"ratio"			"0.65"
		"cooldown"		"420"
		"maxcalls"		"1"
		"recentmaps"	"0"
		"currentmap"	"1"
		"multiple"		"0"
		"override"		""
		"command"		"sm_map {MAP_NAME}"
		"maplist"		"default"
		"start_notify"	"A vote has been started to change level to {MAP_NAME}."
		"pass_notify"	"Vote passed, map changing to {MAP_NAME}"
		"fail_notify"	"Vote failed, current map remains! Received: {VOTE_AMOUNT} Required: {VOTE_REQUIRED}"
		"chattrigger"	"changelevel"
	}

Wait until there are 10 seconds left until map end, initiate a vote. Map changes to initially voted next map, then immediately changes to custom voted map.

Expected behavior
If map vote ends after map ends, it should fail instead of passing without the required votes. This has started happening frequently now that this exploit has been found, sometimes players calling votes as soon as 2 seconds prior to map change to ensure they get the map they want.

Screenshots
N/A

Game Server

• Sourcemod version 1.10-6380 & 1.9-6275
• Metamod version 1.10.7-dev git 968
• Game CS:GO

Additional context
Some logging:

L 01/30/2019 - 20:39:29: [Custom Votes] Vote Changelevel started by User ( STEAM_1:0:xx ). To change map from surf_delusional_ to surf_annoyance_njv.

L 01/30/2019 - 20:31:13: [mapchooser_extended.smx] Voting for next map has finished. Nextmap: surf_akai_final_fix.
L 01/30/2019 - 20:35:13: [SM] Changed map to "surf_akai_final_fix"
L 01/30/2019 - 20:35:14: -------- Mapchange to surf_akai_final_fix --------
L 01/30/2019 - 20:35:14: [basecommands.smx] "Console<0><Console><Console>" changed map to "surf_delusional_"
L 01/30/2019 - 20:35:14: [mapchooser_extended.smx] Loaded map list for csgo.
L 01/30/2019 - 20:35:18: [SM] Changed map to "surf_delusional_"
L 01/30/2019 - 20:35:19: -------- Mapchange to surf_delusional_ --------

As you can see, the map is immediately changed after proper next map is loaded, without even giving clients time to load into the server.

[sneak-it]

This would probably be easiest avoided by cancelling all votes OnMapEnd, correct? I also haven't had a chance to look over the vote evasion commits, but I assume that would be safe as to not improperly punish players who technically "Disconnect" on map change, if a player vote was ongoing.

[caxanga334]

I've added a fix however I was unable to reproduce the issue and test the fix on TF2.

[sneak-it]

I'll go ahead and test the fix this weekend, but it looks like the commit you added should solve the issue!

[sneak-it]

Commit 1be0d5b unfortunately does not cancel votes, as OnClientDisconnect is fired before OnMapEnd. I did a little research and found this thread: https://forums.alliedmods.net/showthread.php?t=168389

I've tested hooking game_end to cancel votes, and at least on CS:GO it seems to not fire depending on server setup, however I've gotten round_end to fire reliably. That being said, perhaps it is worthwhile adding a cvar to cancel votes on round end or game end?

[sneak-it]

Also forgot another factor: At times of an RTV pass and the map changes, this will also cause a client disconnect but not fire any of the above events, so that would also be an issue.

[sneak-it]

Hate to re-open this, however the issue does not seem to be solved on 1.16.3U-dev. On a test server, everything seems fine yet once in production with multiple other players and a much higher longevity, things go awry.

L 03/20/2019 - 02:54:05: [mapchooser_extended.smx] Voting for next map has started.
L 03/20/2019 - 02:54:36: [mapchooser_extended.smx] Voting for next map has finished. Nextmap: surf_ethereal.
L 03/20/2019 - 02:59:17: [SM] Changed map to "surf_ethereal"
L 03/20/2019 - 02:59:17: -------- Mapchange to surf_ethereal --------
L 03/20/2019 - 02:59:17: [basecommands.smx] "Console<0><Console><Console>" changed map to "surf_timber"
L 03/20/2019 - 02:59:21: [SM] Changed map to "surf_timber"
L 03/20/2019 - 02:59:21: -------- Mapchange to surf_timber --------

L 03/20/2019 - 02:41:06: [Custom Votes] Vote Changelevel started by Player 1 ( STEAM_1:0:XXXXXXX ). To change map from surf_funhouse_fix_ to surf_timber.
L 03/20/2019 - 02:59:17: [Custom Votes] Last vote ( Changelevel ) passed. Map was changed from surf_funhouse_fix_ to surf_timber.

This is just one example, it's happened a few times. Under the circumstances of the map change, the game event has been fired, causing a cancel. I'm willing to guess IsVoteInProgress is incorrect, hence the map end cancel vote setting is being ignored.

[sneak-it]

L 03/26/2019 - 19:27:37: [Custom Votes] Vote Changelevel started by Player ( STEAM_1:0:XXXXXXX ). To change map from surf_amplitude_light to surf_2pacisalive.
L 03/26/2019 - 19:38:07: [Custom Votes] DEBUG: Event CSGO_MapEnd. bCancelVoteGameEnd: 1 IsVoteInProgress: 0
L 03/26/2019 - 19:38:22: [Custom Votes] Last vote ( Changelevel ) passed. Map was changed from surf_amplitude_light to surf_2pacisalive.

As you can see here, 11 minutes later (on a normal map change), the vote that failed suddenly gets passed.

Educated myself a bit, IsVoteInProgress is a stock SourceMod function, and given the circumstances the bool value it is returning is correct. I'm really not sure why the map vote is not being invalidated properly upon failure. It seems to work the same as all other votes, as seen here.