Active vulnerability: arbitrary Lua execution in config files

[FPtje]

Apparently this is enough of a problem for someone to release a "hotfix" for it on the DarkRP forums:

http://forum.darkrp.com/threads/ulx-ulib-exploit-fix.9250

The gist of the problem is as follows: ULib simply executes all lines that don't start with exec using game.ConsoleCommand. (See here). Combined with ulx luarun. For malicious addons and superadmins, this is a beautiful feature of persistency for when the addon is uninstalled and the superadmin is demoted.

Reproducing steps

Addon case:

1. Be malicious code in an addon
2. Get installed
3. Write ulx luarun with a RunString and a http.Fetch in the ULX config
4. Have the server owner figure out their mistake and uninstall the addon
5. Malcious code persists until server owner uses programming knowledge and debugging skills to figure out what the fuck's going on

Superadmin case:

1. Be superadmin
2. Run this script, or probably something that doesn't advertise what it's doing so much
3. Act like a massive knob and get demoted
4. Actually you're not demoted because of that persistent Lua code you've put in the config file that no one ever reads.

Now of course all the server owners should have full programming knowledge to debug their problems, know by some force of magic to look into ULib's config files, and they shouldn't make people superadmin if turn out to be non-trustworthy in hindsight. My point is simple:

Config files shouldn't just be read and executed like they're command macros or even Lua files. The fact that they are is being abused.

Solution

With full backwards compatibility, of course.

It looks like ULib.execFile was originially just designed for files in cfg/, which neither superadmins nor malicious addons can write to. The mistake is to use it for ULX' main config, data/ulx/config.txt and its map and gamemode counterparts. Oddly, it appears to be the only file that you execute that way. All other config files seem to be already interpreted normally.

The solution is simple: make a different function for that which just interprets whitelisted commands. Preferably no ulx adduser or ulx luarun. I'd recommend a whitelist because other addons also have persistent concommands (add money to SteamID, pointshop points probably, etc.)

[Nayruden]

Definitely something we'll be addressing, thank you for the report. We continuously caution users to be more careful about what they install on their servers, but we certainly don't want to be a pathway for persistence for an admin's poor choices.

[FPtje]

Thanks!

[Nayruden]

Fixed, closing.