Insecure Bash scripting in HL2 launcher script

[leo-bogert]

Using Steam on Kubuntu Quantal x64, look at the following file: ~/.local/share/Steam/SteamApps/common/Counter-Strike\ Source/hl2.sh

Specifically, at it's usages of "$()": grep '$(' ~/.local/share/Steam/SteamApps/common/Counter-Strike\ Source/hl2.sh

It does the following operations: GAMEROOT=$(cd "${0%/*}" && echo $PWD) ARGSFILE=$(mktemp $USER.hl2.gdb.XXXX)

This is insecure because the output of $() is subject to globbing, word splitting, etc. This means that if the output contains things such as "*", the wildcard WILL be expanded. So you have to add double quotes to the whole $(). Also, any variable expansions inside also need to be double-quoted, i.e. the $PWD and $USER to prevent globbing, etc. for those as well.

Therefore, the correct but non-intuitive solutions to this are the following: GAMEROOT="$(cd "${0%/*}" && echo "$PWD")" ARGSFILE="$(mktemp "$USER.hl2.gdb.XXXX")"

Notice why I say "non-intuitive": The nested double quotes outside the "$()" and inside of it. Someone who came from C-programming will think that the second double quotes will terminate the first one, i.e. that double quotes cannot be nested, but this is NOT the case here: The $( opens a new parsing context, which allows further double quotes to be nested inside it. So the outer quotes "$(...)" quote the whole output of the $() to prevent globbing etc., and the inner quotes quote the ${}, $PWD, etc. to prevent the same there.

Anyone who is new to Bash scripting should read the following pitfall list to learn about such things: http://mywiki.wooledge.org/BashPitfalls It is a very complete list, and commonly referenced on the web.

[Mecha-Weasel]

Nice catch! Hope they fix it!