STEAMROOT computation is tricky and unsafe against whitespaces

[illwieckz]

Your system information

• Steam client version (build number or date): 1538771663
• Distribution (e.g. Ubuntu): Ubuntu 18.04
• Opted into Steam client beta?: Yes but bug occurs on standard Steam too
• Have you checked for system updates?: Yes

Please describe your issue in as much detail as possible:

Talk started in #5819, I was looking for an example of poorly written and unsafe code in Steam's shell scripts so I looked at the very first one and well… the very first line doing something gave me the best example of something wrong:

# figure out the absolute path to the script being run a bit
# non-obvious, the ${0%/*} pulls the path out of $0, cd's into the
# specified directory, then uses $PWD to figure out where that
# directory lives - and all this in a subshell, so we don't affect
# $PWD

STEAMROOT="$(cd "$(dirname $0)" && echo $PWD)"

This is asking for trouble in many ways. The first issue is that it uselessly change directory for nothing. The subshell magic prevents the parent process to be affected but changing a directory like that for nothing is a bad idea by itself because the steam shell can be stored on another filesytem (a removable one, a network one) and doing this make possible for the subshell to be stuck on a non-existent file-system or a network lock if the removable filesystem is just removed at this time or if network is experiencing problems. Also notice that some weird incomplete filesystems are not necessarily walkable or do not have all features you can expect from a filesystem even if those weird incomplete filesystems are fully browsable/readable/writable.

Also, please notice that because of the && logical statement the $STEAMROOT variable would be empty if the directory change does not success. It can happen for example if someone ejects the removable path at the same time. I havent seen any scenario where $STEAMROOT can be set to the parent $PWD (so the directory from where the user calls steam.sh instead of the directory where steam.sh is stored) but computing a foreign path using $PWD is calling for trouble.

Having an empty $STEAMROOT because of special use case already deleted drive of Steam users in the past (see #3671).

Also please notice that if for some unknown reason (I don't see any at his time but…) dirname returns an empty string, and since the behavior of cd is to change directory to $HOME if no argument is given, $STEAMROOT will be set to $HOME, which is not expected at all.

And if for some reason dirname returns a wrong path, current code can't work, and there is reason for this to happens. If dirname fails and returns something wrong, $STEAMROOT will be set to an unexpected value and Steam will run into some troubles. And this can happens since

1. dirname is not called on the absolute path of the script so the directory change can fail in case of weird cross relative symlink hell with many .. involved.
2. $0 is not protected by quotes so dirname is not reliable. For example it means steam won't run if the user home path contains a whitespace character, which is weird but legit.

That is the second issue.

Note that this issue will not only happen if the user home path contains a whitespace character, it will happens too if people move their data outside of their home directory (like people did in #3671) and calls steam.sh directly while the new path has at least one whitespace character in it. And we know people move their data outside so the risk to have a nasty whitespace somewhere is higher than having a nasty whitespace in home path.

The last issue is that the code and related comment are meaningless. This is changing directory to affect $PWD to print it, and the comment just says the code is meant to not change $PWD. Of course while reading the code we understand that the parent's $PWD is not affected, but that's a big problem if people have to read the code to understand the comment. It must be the other way.

Note that the given comment contains some leftovers from previous attempt, since it references a code that does not exist anymore (${0%/*}). The previous iteration featuring ${0%/*} was talked in #3671 too, an issue leading to a filesystem wipe out I experienced myself but was reported by someone else.

Steps for reproducing this issue:

1. make a symlink to ~/.local/share/Steam with a whitespace in it
2. run steam.sh from that symlink
3. you should get something like that:

illwieckz@chroot:~$ ln -s ~/.local/share/Steam 'my steam'
illwieckz@chroot:~$ ./my\ steam/steam.sh 
./my steam/steam.sh: line 22: cd: too many arguments
Couldn't find Steam root directory from ./my steam/steam.sh, aborting!

Note that the same would happens in case of a move (something we know people are doing).

Solution:

# figure out the directory name of the absolute path
# to the script being run

STEAMROOT="$(dirname "$(realpath "${0}")")"

You'll notice that the comment is probably not needed anymore.

It's important to ask for the dirname of the realpath and not the realpath of the dirname because of relative paths and symlinks (and symlinks to symlinks that are relative to relative symlinks) may be found on the way.

Because a wrong value in that $STEAMROOT variable led to massive user file deletion in the past, it can be good to not only check that path is not empty but also validate the computed value before doing anything else, for example by checking if ${STEAMROOT}/steam.sh or another file/directory that must be in ${STEAMROOT} but not elsewhere exists. For example do not check for ${STEAMROOT}/bin because /bin exists and the test would mistakenly success even if ${STEAMROOT} is empty

I appreciate the Linux effort but it looks like you need some help (I still remember the ValveSoftware/SteamOS#177 issue when SteamOS was not able to install Steam 😉).

Unix shell language is very unsafe but since it's the most universal way to go on Linux it's OK to use it but methodology is required. So even if Franck may kill you for that, you must wear both a belt and suspenders because you can't even trust your own pants. 🔫

The shellcheck tool is very helpful but it does not prevent for correct shell code doing things that are not correct for humans. Machines never do what they must do, they always do what we tell them to do, even when we tell them wrong.

[kisak-valve]

Hello @illwieckz, not to summary shoot this down, but the issue with spaces in the path to steam.sh is already being tracked at #3711 and the discussion should continue there. Realpath can not be used at this point because it is not reliably available in all distros. Any distro with that scenario should be able to run steam with STEAM_RUNTIME_PREFER_HOST_LIBRARIES=0 steam for the legacy behavior. From memory, at least Ubuntu 14.04 does not install it by default.

Closing in favor of the existing issue report.