Prevent commands from being run on the client with sm_play#1832
Prevent commands from being run on the client with sm_play#1832peace-maker merged 4 commits intoalliedmodders:masterfrom
Conversation
|
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 |
|
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, |
|
I feel like there is a better way to fix this issue, but I've not got a CSGO dev environment to test with. |
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 |
|
Would another option be using a regex that limits the argument to be a somewhat valid path? It's somewhat debatable what is a fitting regex though, as it can depend on the platform. |
asherkin
left a comment
There was a problem hiding this comment.
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.
|
@b0ink Can you do requested change ? |
|
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. |
* Prevent command injection * Empty to commit to try to kick CI. * Improve filename sanitisation --------- Co-authored-by: Fyren <fyrenmoo@gmail.com> (cherry picked from commit a402b3c)
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_playaccess 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 bothplaygamesoundandsm_banas two individual commands throughClientCommand(""), 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)