Prevent commands from being run on the client with sm_play

[b0ink]

A mate of mine has found that you can force other players to run any command that they have access to (assuming you can target them with !play to begin with) by running !play <target> "\";<command>" in chat. (don't think it has to be that specific, just two quotes and a semicolon before the command)

For example, if I had basic sm_play access as a moderator, and there were no immunity checks on targetting higher admins, I would be able to run !play <target> "\";sm_ban boink 0 reason", and the plugin would be running both playgamesound and sm_ban as two individual commands through ClientCommand(""), on the targetted player.

Checking for a semicolon seems to be the most basic way of preventing this rather than filtering any quotemarks

(Credit to ozaya#7777 for showing me this so that I could make a quick fix for it)

[JoinedSenses]

Which game are you testing on? I don't see the behavior on tf2 that you talk about. The game changes double quotes to single quotes in chat and the double quote isnt escaped in console, so it ends the first command at the semicolon.

[b0ink]

Which game are you testing on? I don't see the behavior on tf2 that you talk about. The game changes double quotes to single quotes in chat and the double quote isnt escaped in console, so it ends the first command at the semicolon.

CS:GO

[Sikarii]

Hey, I was able to replicate this in CSGO, but only through the ingame chat, console does not work.

Seems like CSGO has some quirks with the ingame chat for sure, first";say second in the ingame chat makes 2 messages.
The above will work only on non-vanilla chat processors, as the ingame chat by default has a cooldown, alternatively typing first"; retry on vanilla ingame chat will also work.

[JoinedSenses]

I feel like there is a better way to fix this issue, but I've not got a CSGO dev environment to test with.

[KyleSanderson]

Hey, I was able to replicate this in CSGO, but only through the ingame chat, console does not work.

Seems like CSGO has some quirks with the ingame chat for sure, first";say second in the ingame chat makes 2 messages. The above will work only on non-vanilla chat processors, as the ingame chat by default has a cooldown, alternatively typing first"; retry on vanilla ingame chat will also work.

jeez... ok so there's no proper text handling in the say window (I wonder if the space exploit still works?).

Regardless, while the ingame chat reproduces this easier, it's a legitimate issue. This feels like it needs UTF support based on the current state of games. The present issue is this bypasses ADMFLAG_GENERIC, which is not a good thing for a retail installation.

I think the fix should scan the character behind it to see if it's part of a MB (multibyte / utf) sequence, and if not simply end the string at ;.

[Sikarii]

Would another option be using a regex that limits the argument to be a somewhat valid path?
One candidate could be: ^[a-zA-Z0-9\/_-]+(?:\.[a-zA-Z0-9]+)*$, or limiting the entire string to [a-zA-Z0-9\/_-\.] or similar.

It's somewhat debatable what is a fitting regex though, as it can depend on the platform.
Regardless, I think this issue should be prioritized, this is privilege escalation at it's finest.

[asherkin]

This is a good start but I think it could both trigger unexpectedly (semicolon in target string) and miss some cases of badness (quotes in the path).

I'd say we want a check for ; or " being present after we've done both the argument parsing and quote stripping - so using Arguments[len] as the input to StrContains right before (or after) we do the ProcessTargetString.

[Rushaway]

@b0ink Can you do requested change ?

This has been addressed

[peace-maker]

I think going the stricter approach of removing and blocking any dangerous characters is better than trying to keep multi-byte characters which happen to contain one of the bad char bytes intact. If there are really soundfiles with a quote or semicolon in it's name, maybe the better solution is to rename the file.