DavidGriffith / frotz

Infocom-style interactive fiction player for Unix and DOS (moved to https://gitlab.com/DavidGriffith/frotz)
GNU General Public License v2.0
209 stars 64 forks source link

Saving can overwrite arbitrary files #46

Open Roadcrosser opened 7 years ago

Roadcrosser commented 7 years ago

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:

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

I see. Thanks for bringing up this problem.

DavidGriffith commented 7 years ago

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

DavidGriffith commented 7 years ago

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

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.

Roadcrosser commented 7 years ago

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

DavidGriffith commented 7 years ago

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