Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is the release script safe ? #46

Closed
xprov opened this issue Jun 7, 2021 · 6 comments
Closed

Is the release script safe ? #46

xprov opened this issue Jun 7, 2021 · 6 comments
Labels
GameShell relative to GameShell code missions relative to particular missions

Comments

@xprov
Copy link

xprov commented Jun 7, 2021

Line 77 of gameshell.sh is rm -rf "$GSH_ROOT" where $GSH_ROOT is defined as $ORIGINAL_DIR/$GSH_NAME.

Can there be a scenario leading to both $ORIGINAL_DIR and $GSH_NAME definitions to fail, resulting in the execution of rm -rf / or something similar ? It might be a good idea to check that $GSH_ROOT is different from / and ./.

Note that such a thing did happen with Valve's Steam install script : https://en.wikipedia.org/wiki/List_of_software_bugs#Video_gaming

@xprov xprov changed the title Is the install script safe ? Is the script safe ? Jun 7, 2021
@xprov xprov changed the title Is the script safe ? Is the release script safe ? Jun 7, 2021
@phyver
Copy link
Owner

phyver commented Jun 7, 2021

Ah!
I don't see how that could happen, but it doesn't mean it won't.
For now, I added a couple of safeguards to prevent disasters. (I check that the directory contains the start.sh script and the missions directory.)

But similar safeguards shoud probably be added in other places as well! Several missions do use rm -rf...

Let's keep that open.

@phyver phyver added GameShell relative to GameShell code missions relative to particular missions labels Jun 7, 2021
@rlepigre
Copy link
Collaborator

rlepigre commented Jun 7, 2021

Makes me wonder: can't we somehow set things up to run the game in a chroot?
That would not be without problems though:

  • We'd still need to add links to directories of the PATH and certainly more things.
  • You probably cannot run the chroot system call as a "normal" user.

@phyver
Copy link
Owner

phyver commented Jun 8, 2021

That will probably open a whole lot of problems!
A safeguard that is easy to implement is

  • add a hidden .gsh_root file in $GSH_ROOT, and check it exists before rm -rf
  • rename World, .config, .var, etc. to gsh_World,.gsh_config, .gsh_var, etc. and match any directory we want to rm -rf with */*gsh*. We could even do something a little more robust, in case user "Guillaume Shirt" (login gshir) wants to play...

@Mte90
Copy link
Contributor

Mte90 commented Jul 4, 2022

Probably a chroot is a safer way and can avoid #99 this ticket

Or maybe every time the rm is executed check if there is that hidden file .gsh_root or other files.

@phyver
Copy link
Owner

phyver commented Jul 5, 2022

chroot would bring its share of problems. Since we rely on the system's utilities, we would need to have link inside GameShell to access them. Doing so in a robust and portable way is probably difficult.

I could add a scripts/rm that checks we are removing part of GameShell's filesystem hierarchy is probably possible. Something like

#!/bin/sh

GSH_ROOT=$(cd "$(dirname "$0")/.." && pwd -P)

for arg in "$@"
do
  case "$arg" in
    -*)
      continue
      ;;
  esac
  case "$("$GSH_ROOT"/scripts/readlink-f "$arg")" in
    "$GSH_ROOT"*)
      continue
      ;;
    *)
      echo "safe_rm: cannot remove '$arg': it is not part of GameShell" >&2
      exit 1
      ;;
  esac
done

/bin/rm "$@"

seems to work, but should probably be tested a little more.
If any argument is outside GameShell's directory, nothing is done and an error message is written on stderr.

This offers some protection for uses of rm from GameShell's scripts and missions, and for interactive uses of rm from the player.

Of course, it doesn't help if the player redefines GSH_ROOT, but we have to stop worrying at some point.

@phyver
Copy link
Owner

phyver commented Jul 11, 2022

9479630 adds a "safe" rm script that does just that.

It doesn't guarantee the release script is safe, but I don't think there can be such a guarantee!

@phyver phyver closed this as completed Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GameShell relative to GameShell code missions relative to particular missions
Projects
None yet
Development

No branches or pull requests

4 participants