Steaming!

Valve’s steam appears to be a package manager for installing Valve software (games). Part of steam on Linux is a shell script: steam.sh.

It turns out, if you’re not careful, if you try and uninstall steam or something… then this innocent 600 line shell script can kind of accidentally DELETE ALL YOUR USER FILES. Ha ha.

Much hilarity in the github issue.

At core the proximate issue is executing this command:

	rm -rf "$STEAMROOT/"*

The problem is that, perhaps in mysterious circumstances, STEAMROOT can be set to the empty string. Which means the command rm -fr "/"* gets executed. Which removes all the files that you have access to on the system (it might take its time doing this).

I’m working off this version of steam.sh.

First off, it’s 600 lines long. That, right there, should set the alarm bells ringing. No shell script should be that long. It’s just not a suitable language for writing more than a few lines in.

set -u, whilst a good idea in a lot of scripts, would not have helped here. As it happens, STEAMROOT is set, but set to the empty string.

"${STEAMROOT:?}", as suggested by one of the commentor’s in github, would have helped. The script will exit if STEAMROOT is unset or set to the empty string.

Immediately before this code there is a comment saying “Scary!”. So that’s another thing. If one programmer thinks the code is scary, then we should probably review the code. And make it less scary. Clearly adding an explicit check that STEAMROOT is set would have helped make it less scary.

It would also be a good idea to add a -- argument to rm to signify the end of the options. Otherwise if STEAMROOT starts with a «-» then it will trigger rm into thinking that it is an option instead of the directory to delete. So we should write:

    rm -fr -- "${STEAMROOT:?}"/*

STEAMROOT is assigned near the beginning of the file:

STEAMROOT="$(cd "${0%/*}" && echo $PWD)"

It is often problematic to use command substitution in an assignment. The problem being that the command inside the round brackets, cd "${0%/*}" && echo $PWD in this case, could fail. The shell script still carries on and assigns the stdout of the command to the variable. And if the command failed and produced no output then STEAMROOT will become the empty string.

Here would be a good place to explicitly check that STEAMROOT is not an empty string. : "${STEAMROOT:?}" will do, but if [ -z "$STEAMROOT" ] ; then exit 99; fi is more explicit.

set -e would have helped. If a command substitution is assigned to a variable and the command fails (exit code != 0) then the assignment statement fails and that will trigger set -e into exiting the script. It’s not ideal error checking, but it is better than nothing.

The code, as described by the comment above it, is trying to find out the location of the script. This is often problematic. There’s no portable way to find out. But as long as you’re in bash, and the script is explicitly is a bash script and uses various bashims, why not just use the relatively straightforward DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) as recommended by this Stack Overflow answer. No need to pretend that $0 is set to anything useful. (all of the above still applies though)

The steam.sh script is a bit enigmatic. Bits of it are written by someone who clearly knows shell scripting. The "{$0%/*"} thing to strip off the last component of a path is not common knowledge. But why not use dirname as the code later on in the script does? Correctly uses the portable equality operator single «=» in code like if [ "$STEAMEXE" = "steamcmd" ], but later on uses the bashism «==» and «[[». Clearly knows about the $( ... ) notation for command substitution, but then uses legacy (and yukhy) backquote syntax elsewhere. Carefully avoids using dirname (in the POSIX standard, and therefore very likely to be installed on any given Unix system), but then uses curl without checking (and curl isn’t installed on Ubuntu by default).

In summary: too long; attempting to locate directory containing script is problematic; doesn’t do enough checking (in particular, set -e).