MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001426Zandronum[All Projects] Suggestionpublic2013-07-22 23:502016-01-26 12:38
ReporterDusk 
Assigned To 
PrioritynormalSeverityexploitReproducibilityN/A
StatusnewResolutionopen 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version 
Summary0001426: Make ConsoleCommand whitelist-based
DescriptionIn light of all these exploits and abuse going about, I thought I'd put this on the table: Make ConsoleCommand whitelist-based. Instead of blacklisting the bad commands, white-list the ones we want ConsoleCommand to use until we're ready to cut it off. It'd be at least a partial fix to this security issue.

Further it'd also give us a list of stuff we need to allow ACS to do before we can finally say bye bye to ConsoleCommand - while we patch the use cases, we could prevent new ones from arising. I'm not too sure exactly what ones need to be set. Also I'd suggest changing the new cvar flag to CVAR_ALLOWSETBYACS or something like that to do the same whitelisting to cvars.
Steps To Reproduce
Additional Information... because y'know
'http://pastebin.com/f6Z5c2L9 [^]'
this is like sort of like kinda bad
TagsNo tags attached.
Relationships
related to 0001070new Dusk Phasing out the dangerous ConsoleCommand for various additional/improved ACS commands 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2013-07-22 23:50DuskNew Issue
2013-07-22 23:51DuskDescription Updatedbug_revision_view_page.php?rev_id=3761#r3761
2013-07-22 23:53DuskRelationship addedrelated to 0001070
2013-07-23 00:00DuskNote Added: 0006762
2013-07-23 06:53Blzut3Note Added: 0006764
2013-07-23 12:02DuskView Statusprivate => public
2013-07-23 12:07DuskNote Added: 0006768
2013-07-23 12:09DuskDescription Updatedbug_revision_view_page.php?rev_id=3762#r3762
2013-07-23 14:47ZzZomboNote Added: 0006772
2013-07-23 16:59Konar6Note Added: 0006777
2013-07-23 17:55DuskNote Added: 0006778
2013-07-23 18:38DuskNote Edited: 0006778bug_revision_view_page.php?bugnote_id=6778#r3766
2013-07-23 18:38DuskNote Edited: 0006778bug_revision_view_page.php?bugnote_id=6778#r3767
2013-07-23 20:33CatastropheNote Added: 0006780
2013-07-23 20:35CatastropheNote Edited: 0006780bug_revision_view_page.php?bugnote_id=6780#r3769
2013-07-23 20:35CatastropheNote Edited: 0006780bug_revision_view_page.php?bugnote_id=6780#r3770
2013-07-23 20:41DuskNote Added: 0006781
2013-07-23 20:41DuskNote Edited: 0006781bug_revision_view_page.php?bugnote_id=6781#r3772
2013-07-23 20:41DuskNote Edited: 0006781bug_revision_view_page.php?bugnote_id=6781#r3773
2013-07-24 19:33Torr SamahoNote Added: 0006787
2013-08-07 09:25EsumNote Added: 0006921
2013-08-08 17:43DuskNote Added: 0006944
2013-08-12 07:22CatastropheNote Added: 0006998
2013-08-12 14:10DuskNote Added: 0006999
2013-11-29 17:32HypnotoadNote Added: 0007635
2013-11-30 09:50GezNote Added: 0007642
2013-11-30 09:51GezNote Edited: 0007642bug_revision_view_page.php?bugnote_id=7642#r4238
2013-11-30 12:00HypnotoadNote Added: 0007643
2014-06-15 14:46WatermelonNote Added: 0009392
2015-06-21 18:18arkoreNote Added: 0012772
2015-06-21 18:18arkoreNote Edited: 0012772bug_revision_view_page.php?bugnote_id=12772#r7523
2016-01-26 12:34DuskSeveritymajor => exploit

Notes
(0006762)
Dusk   
2013-07-23 00:00   
I wonder, should this be a public ticket? Could get some help building the possible white-list that way.
(0006764)
Blzut3   
2013-07-23 06:53   
I see no reason this has to be private. In fact, I think it would be better that it isn't as it serves as a good example for all the people who think that consolecommand is harmless. :P

As you can probably guess, I'd be all for a white list or even making it only available as a compat flag.
(0006768)
Dusk   
2013-07-23 12:07   
Made public. I don't think that compatflagging is sufficient since a mod could just say it needs compat_acsconsolecommand true or something like that and people who have no idea what this even is would just activate it, and we have the same problem all over again. IMO it should be completely torn out.

So yeah. I think a list of things we should allow for now...
- set... maybe only if the cvar being set is mod-generated? Also AOW and WDI auto-adjust DMFlags and nobody complains.
- archivecvar, same rationale as above
- puke? Might be needed to get some info from clients to servers...
- kick? I don't know any legit uses off-hand though
(0006772)
ZzZombo   
2013-07-23 14:47   
Kick a troll armed a C4/Ion/Nuke in his base.
(0006777)
Konar6   
2013-07-23 16:59   
The attached snippet is part of a wad which one user was trying to host on servers. The full wad is here ->'http://sickedwick.net/k6/9ab7f5118db665ff6e2cc7dea7d4e2e9/surprise_me_v1.pk3_ [^]' (WARNING: don't run it, it's a malware that will screw your config if you do; I've renamed it to prevent accidental execution).

This shows how dangerous ConsoleCommand still is, despite of removing specific reported commands one by one. It's simply not enough even in the short run to foresee and catch all possible misuse. I would also like to remind that a huge security issue arising from ConsoleCommand, which allowed a wad to delete arbitrary files from a remote server by modifying sv_*lists values, has been only recently brought to dev's attention and fixed.

I fully support the idea behind this ticket, I feel that ConsoleCommand is overrated and that the real legit use is minimal.

> - kick? I don't know any legit uses off-hand though
IMHO a wad shouldn't meddle with server actions like this. There is already KickFromGame, which I believe is enough to have.
(0006778)
Dusk   
2013-07-23 17:55   
(edited on: 2013-07-23 18:38)
Quote

> - kick? I don't know any legit uses off-hand though
 IMHO a wad shouldn't meddle with server actions like this. There is already KickFromGame, which I believe is enough to have.

That's what I had in mind too. A mod shouldn't really have much a need anyway for the kick command; auto-kicks are just bad because they can greatly disrupt the game over a preset set of conditions which always has corner cases.
(0006780)
Catastrophe   
2013-07-23 20:33   
(edited on: 2013-07-23 20:35)
"set", "archivecvar", and "puke" needs to be whitelisted if you guys are doing this.
(0006781)
Dusk   
2013-07-23 20:41   
That's kind of exactly what I outlined here.
(0006787)
Torr Samaho   
2013-07-24 19:33   
I think this is one of the things we should probably also discuss on the forum since a lot more people are reading those than the tracker.

BTW: As I already stated on multiple occasions, I fully agree that ConsoleCommand is an abomination that needs to go.
(0006921)
Esum   
2013-08-07 09:25   
I suggest to whitelist eval and test as well unless they trigger commands that are not whitelisted.
(0006944)
Dusk   
2013-08-08 17:43   
bump - just today we caught another case of abuse on the BE cluster. IMO we should hurry up with this and push out a security release.

I don't know what'd eval and test be useful for here - can't GetCVar work just as well?
(0006998)
Catastrophe   
2013-08-12 07:22   
I also use +Attack to "wake up" monsters in a map; is there an alternative to this?
(0006999)
Dusk   
2013-08-12 14:10   
Quote
I also use +Attack to "wake up" monsters in a map; is there an alternative to this?


'http://zdoom.org/wiki/NoiseAlert [^]'
(0007635)
Hypnotoad   
2013-11-29 17:32   
>I feel that ConsoleCommand [...] real legit use is minimal.

I strongly disagree. Due to some major obstacles with acs many, many mods are completely reliant on consolecommand at present. You'll be hard pressed to find any complex acs mod on zandronum that doesn't use consolecommand to some degree.
(0007642)
Gez   
2013-11-30 09:50   
(edited on: 2013-11-30 09:51)
""set", "archivecvar", and "puke" needs to be whitelisted if you guys are doing this."

Why should "puke" be whitelisted? You can already use the ACS_*Execute* commands from within ACS. This means that puke is redundant, superfluous, unneeded, and extraneous.
(0007643)
Hypnotoad   
2013-11-30 12:00   
Gez, puke is used because one of the many limitations of the engine is the inability to send information about variables etc changed clientside back to the server. Therefore one must puke a script from the client with arguments so that it can be executed on the server.
(0009392)
Watermelon   
2014-06-15 14:46   
While I support the idea a lot, we'd probably have to see what the major mods use this for and try to get a list from there so we'd have a good head start.
(0012772)
arkore   
2015-06-21 18:18   
I don't know if BFH is major, but it uses consolecommand() for loading a new map (not changing map), since there's no other way to do it. The exit portal at the end will execute consolecommand("MAP START") to restart the mod.