Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Saving can overwrite arbitrary files #46

Open
Roadcrosser opened this issue Jun 17, 2017 · 13 comments
Open

Saving can overwrite arbitrary files #46

Roadcrosser opened this issue Jun 17, 2017 · 13 comments
Assignees

Comments

@Roadcrosser
Copy link

Roadcrosser commented Jun 17, 2017

I run a Discord Bot that runs games on instances of DumbFrotz. However, when saving a game, one can specify an arbitrary filepath for the save, potentially overwriting other files in the entire filesystem.

Examples of dangerous filepaths one can specify:

  • /home/pi/Desktop/save
  • ../save

Replacing save with any filename will simply overwrite them.

Suggestions:
A flag when initializing DFrotz to output the raw savedata to STDOUT to be read externally by the program managing it.

@DavidGriffith
Copy link
Owner

I hope you're not running DumbFrotz as a user that has write privileges anywhere sensitive.

Good idea though. I'll see what I can cook up this weekend. An easier thing to implement would be to specify that we should always save the game with a given filename. The managing program could then monitor that file for changes and then move or delete it as needed. What do you think?

@Roadcrosser
Copy link
Author

Sounds good, I can check whether any new save files appear after each command is issued and upload before deleting.

I ought to suggest something similar for restoring, but I should do that in another issue.

@DavidGriffith
Copy link
Owner

There are two other potential problems: script files and command recordings. How about this: A flag takes a directory path as a parameter. If this flag is used, then all files created will go there with no additional paths allowed, that is, cut off everything to left of a slash including the slash itself. This should keep users from traipsing about the filesystem.

@Roadcrosser
Copy link
Author

Roadcrosser commented Jun 17, 2017

I am unsure of what script files or command recordings are.

Forcing a directory as a flag should also work, although it would need me to create a folder for every game that is started (since multiple games can run concurrently), but I should still be able to detect, upload and delete anything in each folder.

@DavidGriffith
Copy link
Owner

DavidGriffith commented Jun 17, 2017

The Z-machine provides mechanisms for creating a transcript of the game and for recording the commands used. The commands SCRIPT ON, SCRIPT OFF, RECORDING ON and RECORDING OFF are used to toggle these modes. When script mode is turned on, whatever appears on the screen will be saved into a file, which is by default the filename followed by .scr. For recording mode, only what is typed by the player will be saved. It's default is the filename followed by .rec. These are plain text files. A recording file can be replayed with the REPLAY command.

@Roadcrosser
Copy link
Author

I see. This should work then, the save-detector can ignore .scr and .rec files, only uploading them when the game ends and the folder is cleared out.

@DavidGriffith
Copy link
Owner

Or clear them out when the user is finished playing. Suppose the user reaches an undesirable ending and wants to load a save file.

@Roadcrosser
Copy link
Author

Roadcrosser commented Jun 17, 2017

The game would end when the user chooses to quit or runs a forcequit that terminates the process manually.

I have plans for a rudimentary filestorage system that allows uploading/downloading scr/rec/saves that last between the time the game starts and ends. (or just only upload saves because of how complicated that might get)

@DavidGriffith
Copy link
Owner

I see. Thanks for bringing up this problem.

@DavidGriffith
Copy link
Owner

Please take a look at 5432358 and tell me if it meets your requirements.

@DavidGriffith
Copy link
Owner

DavidGriffith commented Jun 28, 2017

Restricted read/write now in both curses and dumb interfaces: a14dc90

Invoke it with the -R flag. For instance, dfrotz -R /home/ifbot/output zork1.z3.
The exact path provided should vary with each chat instance to prevent users from stomping on each others' saves.

Modification for DOS interface pending.

@DavidGriffith DavidGriffith self-assigned this Jul 1, 2017
@Roadcrosser
Copy link
Author

I can't read C, but with the description, that should suffice.

@DavidGriffith
Copy link
Owner

Oops! I just realized that my restriction code doesn't actually restrict reading.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants