An sv_filtercommands option that doesn't filter ; in chat

[Caelish]

At the moment, the sv_filtercommands cvar (when set to 1) filters \r, \n, and ;. That's great, but comes with the caveat of legitimate uses of semicolons in chat being filtered. It'd be really nice to have a weaker setting for it where it doesn't filter ; in chat (other places seem fine to me), so there's no impact noticeable to players while still filtering e.g. \r and \n, as well as uses of ; outside of chat messages.

The idea is to have an option where it's weakened to the point where it's still better than not having it, but is not noticeable to legitimate players.

[ensiform]

It becomes more expensive to start doing string look-ups based on commands used especially since some mods use non-standard alternate chat too as well. But its something to look at.

This option is only necessary for JA+ mod or original basejka jampgame FYI in terms of still used mods and possibly OJP. JA++, OpenJK, all have the issues fixed in their modcode which is why it is defaulted to 0.

[ensiform]

I have a working proof of concept that does some other cool things for sv_filterCommands with options for a few specific cases now.

[Caelish]

Sounds awesome. Unfortunately, like 80% of my customers want to use JA+ regardless of its security issues, so the cvar is still more useful to me than I'd like.

[ensiform]

See the referenced posted I made in the jk2mv thread about current implementation ideas.

[mrwonko]

It becomes more expensive to start doing string look-ups based on commands used especially since some mods use non-standard alternate chat too as well.

When you get no more than say 10 chat messages a frame that should hardly be an issue.

Oh wait, this is all commands combined, right? Still, I doubt it's a bottle neck, but I'd have to test.

[ensiform]

I'm also looking at fixing some other things that old mods aren't fixed either but it will be limited scope.

[Caelish]

That implementation looks much, much better than the existing one. I currently have it disabled because of complaints, I'd absolutely re-enable it in that form.

One thing I'm fuzzy on, does your implementation still filter ; in chat-and-everywhere-else if set to 2? Security-wise, it might be nice to have an option to continue filtering ; everywhere as well, with specific exceptions for say, say_team, and tell. Even if slightly expensive, it could be good in case of similar exploit vectors besides the callvote one that we don't know about.

Something like...

define SVFC_GENERAL_NORMAL 1 //Filter \r and \n

define SVFC_GENERAL_STRICT 2 //Filter \r \n and ; except in chat messages

define SVFC_GENERAL_PARANOID 4 //Filter \r \n and ;

define SVFC_FIX_CHAT 8

define SVFC_FIX_CALLVOTE 16

define SVFC_FIX_BADTEAMS 32

Or...

define SVFC_GENERAL_NORMAL 1 //Filter \r \n and ;

define SVFC_FIX_CHAT 2 //Filter \r and \n in chat

define SVFC_FIX_CHAT_STRICT 4 //Filter \r \n and ; in chat

define SVFC_FIX_CALLVOTE 8

define SVFC_FIX_BADTEAMS 16

Or is that already possible in your existing implementation?

[ensiform]

Currently the strict is meant to be strict everywhere including the chat. Tbh the ; check is not necessary anywhere but callvote afaik.

The only issue is that several mods might still have vulnerabilities with custom commands which makes it hard to predict the buffer size limits. Currently chat cuts each one off at 150 but need to figure out how to limit that to full buffer in engine. And everything else is 256 (cvar value length)

I'll consider the paranoid option though. But I'm not sure it will work as expected because currently all commands are checked early on. And the general check happens later just before sending the command to mod code.

Edit: Found possible solution.

Updated gist: https://gist.github.com/ensiform/4e1c418a25ebfd47c25c

[Caelish]

That updated one is perfect and amazing and great. All of my approval is yours.

[ensiform]

But we don't really like the idea of having gamecode stuffs in engine. So an extra layer API would be nice.

[ensiform]

@Razish please close this when your change gets merged to master.

[Razish]

Oops, looks like it was fixed in 8b07e58