[0.16.51] Exploit to create scenario folders anywhere on client's file system

[grilledham]

Attached is a save that when loaded will create a folder C:\free_candy_machine at least on my windows 10 machine.

Steps to make the save:
1. Create a scenario folder at the target location C:\free_candy_machine
2. Launch factorio with the scenario using a relative path

Code: Select all

factorio.exe --start-server-load-scenario /../../../../../../../free_candy_machine

3. Close server and delete scenario folder from step 1.
4. Load the save free_candy_machine.zip

Interestingly if you try to create the save file with

Code: Select all

factorio.exe -m /../../../../../../../free_candy_machine

Factorio claims the scenario can't be found.

[Muppet9010]

The end user impact was that a scenario file from the Redmew server (with their custom path) wasn't compatible with our server. Our server runs on a vanilla path and has a typical locked down access to the file structure outside of the factorio data folder.

We would have thought that the scenario file name is local under the path of the server and wouldn't include the modified path in its name. Then each server/client can be configured as desired and everything is compatible?

[TruePikachu]

I can reproduce under Windows 7 as well.

This is dangerous enough as it is, as being able to place a file controlled by the attacker in an arbritrary location, even if creating a new directory in the process and the file going in there, is dangerous. For instance, if it is used to create an entry in the Start Menu's "Startup" directory, the created folder will be opened the next time the user (or any, if placed under All Users) logs in; if it contains a specially-crafted `desktop.ini` file, or is named with a CLSID, it can potentially cause undesired effects on the target system.

[Rseding91]

Thanks for the report. It's now fixed for 0.17.

--start-server-load-scenario does not support file paths in any part of the input arguments and will treat the entire input string as the scenario name.

[TruePikachu]

A carefully-crafted `level.dat` (e.g. from memory manipulation) can likely still cause the issue, if the only fixes made are what you just described.

[Rseding91]

A carefully-crafted `level.dat` (e.g. from memory manipulation) can likely still cause the issue, if the only fixes made are what you just described.

It can't. The game checks both times that the level name is just a name and not some relative/non-relative path.