phyver / GameShell

a game to learn (or teach) how to use standard commands in a Unix shell
GNU General Public License v3.0
2.17k stars 138 forks source link

Is the release script safe ? #46

Closed xprov closed 2 years ago

xprov commented 3 years ago

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

phyver commented 3 years ago

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.

rlepigre commented 3 years ago

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

phyver commented 3 years ago

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

Mte90 commented 2 years ago

Probably a chroot is a safer way and can avoid https://github.com/phyver/GameShell/issues/99 this ticket

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

phyver commented 2 years ago

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

9479630e 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!