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

Backslashes are not escaped when running processes through Reaper #731

Open
vadcx opened this issue Dec 21, 2024 · 3 comments
Open

Backslashes are not escaped when running processes through Reaper #731

vadcx opened this issue Dec 21, 2024 · 3 comments

Comments

@vadcx
Copy link

vadcx commented Dec 21, 2024

Your system information

  • Steam Version: 1733265492
  • Steam Runtime Version: 0.20241024.105847
  • Distribution (e.g. Ubuntu 18.04): Manjaro
  • Link to your full system information: ---
  • Have you checked for system updates?: No
  • What compatibility tool are you using?: None / Proton 9.0 (both tested, this is minimal repro)
  • What versions are listed in steamapps/common/SteamLinuxRuntime/VERSIONS.txt:
depot	0.20240806.0			# Overall version number
LD_LIBRARY_PATH	-	scout	-	# see ~/.steam/root/ubuntu12_32/steam-runtime/version.txt
scripts	0.20240806.0			# from steam-runtime-tools
  • What versions are listed in steamapps/common/SteamLinuxRuntime_soldier/VERSIONS.txt:
depot	0.20241118.108551			# Overall version number
pressure-vessel	0.20241118.0	scout		# pressure-vessel-bin.tar.gz
scripts	0.20241118.0			# from steam-runtime-tools
soldier	0.20241118.108551	soldier	0.20241118.108551	# soldier_platform_0.20241118.108551/
  • What versions are listed in steamapps/common/SteamLinuxRuntime_sniper/VERSIONS.txt:
depot	0.20241118.108552			# Overall version number
pressure-vessel	0.20241118.0	scout		# pressure-vessel-bin.tar.gz
scripts	0.20241118.0			# from steam-runtime-tools
sniper	0.20241118.108552	sniper	0.20241118.108552	# sniper_platform_0.20241118.108552/

Please describe your issue in as much detail as possible:

Launch Options should escape backslashes to pass them through properly. Mostly used for Wine/Proton apps when a file path needs to be specified.

Bash's printf "%q" would do, but it's not part of POSIX (man printf.1p). Does Steam care about POSIX shells?

PS: I hope this is in the correct repository

Steps for reproducing this issue:

  1. Add non-steam game: /usr/bin/cat
  2. Launch options (GNU cat) to make it stall but also showcase the args: -- /dev/zero --doubledash forward/slash back\slash double\\backslash
  3. Observe the args:
pgrep cat
# shows PID
ps -ewwo pid,etime,cmd $PID
  17962       01:50 /home/HOME/.local/share/Steam/ubuntu12_32/reaper SteamLaunch AppId=3378455283 -- /usr/bin/cat -- /dev/zero --doubledash forward/slash backslash double\backslash
  17963       01:49 /usr/bin/cat -- /dev/zero --doubledash forward/slash backslash double\backslash

Expected:

/usr/bin/cat -- /dev/zero --doubledash forward/slash back\slash double\\backslash

Actual:

/usr/bin/cat -- /dev/zero --doubledash forward/slash backslash double\backslash

@smcv
Copy link
Contributor

smcv commented Jan 2, 2025

This is certainly not a Steam Runtime bug: the Launch Options are interpreted by the Steam client, not the Steam Runtime, so if it's a bug in anything, it would be ValveSoftware/steam-for-linux.

I think this is working as intended, although the design is not what you thought it was. The arguments in Launch Options are interpreted as though they were appended to the command as part of a shell script, so in particular they use and require sh(1) quoting or escaping where appropriate. Quite a lot of Steam users already rely on that being true, so it's unlikely to be changed now.

If you think about it for a while, you'll realise that there needs to be some sort of defined syntax here, because the input box takes a single string, but you want to be able to pass more than one argument, which means there needs to be some way to split the string into individual arguments - but Steam can't just split at whitespace, because then you wouldn't be able to pass an argument that contains whitespace. So the choices are to either adopt shell quoting syntax as-is, or reinvent it (most likely badly, and certainly with less documentation). The Steam developers have chosen to adopt shell quoting syntax, rather than reinventing it.

This means that if you want to pass arbitrary user-specified arguments to a game via its Launch Options, then yes, you do need to quote those arguments according to POSIX sh(1) rules, so arguments with backslashes will need either single quotes or doubled backslashes. You can do this programmatically with bash's printf '%q', or with external library code like Python shlex.quote or GLib g_shell_quote() (in practice putting each argument in single quotes will be sufficient in nearly all cases).

The final command that Steam actually runs is constructed from the Launch Options, some wrappers (reaper and steam-launch-wrapper), compatibility tools if any, the absolute path to the game, and the game's configured executable. Steam correctly adds quotes where necessary when interpolating those paths.

More precisely, setting Launch Options to some arguments like -- /dev/zero ... (as in your example) is syntactic sugar for setting the Launch Options to %command% -- /dev/zero ..., where %command% gets substituted by Steam as the result of combining wrappers, compatibility tools and game executable, all shell-escaped where necessary; and then the whole thing gets run as a one-line shell command by invoking {"sh", "-c", "...", NULL} as an argv array. You can see the actual command in one of Steam's log files, with \0 between items of the argv array - I think it ends up in console_log.txt at the moment?

For non-Steam games, a somewhat simpler code path is used (for example steam-launch-wrapper is missing) but the general operating principle is the same.

All of the wrappers and compatibility tools "below" the Steam client are careful to pass through their argv as an array of arguments, so no further quoting or unquoting is needed at that level. You should apply exactly one level of sh(1) quoting to any arguments that are specified in Launch Options, and then the right thing will happen.

Bash's printf "%q" would do, but it's not part of POSIX (man printf.1p). Does Steam care about POSIX shells?

Steam runs the final composite command via sh -c 'blah blah blah', and /bin/sh is not always bash (notably, it is not bash on Debian/Ubuntu), so no, Steam cannot rely on printf '%q' working the way it does in bash. However, on your specific computer with your specific environment, if you happen to know that your /bin/sh is bash, you can rely on bashisms if you want to.

@TTimo
Copy link
Collaborator

TTimo commented Jan 2, 2025

Steam runs the final composite command via sh -c 'blah blah blah', and /bin/sh is not always bash (notably, it is not bash on Debian/Ubuntu), so no, Steam cannot rely on printf '%q' working the way it does in bash. However, on your specific computer with your specific environment, if you happen to know that your /bin/sh is bash, you can rely on bashisms if you want to.

Speaking of which, I wonder if we should change this to /bin/bash and make bash a requirements? We run into bash/dash problems every so often, and it seems like we could avoid that whole class of problems.

@smcv
Copy link
Contributor

smcv commented Jan 2, 2025

Steam runs the final composite command via sh -c 'blah blah blah', and /bin/sh is not always bash

Speaking of which, I wonder if we should change this to /bin/bash and make bash a requirements? We run into bash/dash problems every so often, and it seems like we could avoid that whole class of problems.

Perhaps, but we should be careful. Everyone (even NixOS) seems to agree that /bin/sh needs to exist, but the same is not true for /bin/bash: less-usual operating systems sometimes only have something like /nix/blahblah/bin/bash or /usr/local/bin/bash.

I believe Steam on Linux already does require bash to be installed somewhere in the PATH (because it has #!/usr/bin/env bash scripts), so it could potentially run games using bash and a PATH search, if you want to. If it's currently calling execv, execve or posix_spawn, you'd need to replace that with execvp, execvpe or posix_spawnp respectively, to enable PATH search, or if it's using the g_spawn_async() family you'd need to make sure G_SPAWN_SEARCH_PATH is enabled, or the equivalent for other API families.

system(), popen() and other simple shell entry points are typically hard-coded to use /bin/sh or sh as the interpreter, but hopefully Steam is using argv-based APIs instead of those.

(We do guarantee the presence of /bin/bash inside a Steam Linux Runtime container, so we can use #!/bin/bash for scripts that never run outside a container, like the Steam Linux Runtime 1.0 scout-on-soldier entry point, but we're careful to avoid that for scripts that run on the host system.)

Reasons to prefer bash:

  • more featureful (notably it has local, set -o pipefail, arrays other than "$@", and printf '%q')
  • somewhat consistent feature-set between Debian/Ubuntu (sh is dash) and Fedora/Arch/etc. (sh is bash)
  • consistent feature-set between your interactive shell and shell scripts, if your interactive shell happens to also be bash
  • has the optimization where trivial commands implicitly behave as if prefixed with exec
  • has some other optimizations, such as more in-process builtins

Reasons to prefer sh even if it might be dash:

  • smaller, therefore faster startup for the common case of trivially short scripts
  • dash only depends on glibc (doesn't need libtinfo like bash does), therefore is immune to some library-loading issues that can affect bash
    • I've wondered whether to mitigate this by linking the Steam Runtime's bash to a static libtinfo
  • its guaranteed feature-set is woefully small but hasn't changed for 20+ years, so we're more likely to write scripts that work on oldLTS operating systems from the distant past
  • consistent with the feature-set you'll see from system() and popen()
  • what you explicitly ask for is what you get, no hidden optimizations
  • if bash features are available then developers will be tempted to use them, and it's easy to end up with a large bash script that would have been more maintainable if written in C/C++/Python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants