7 ms·
Here's the offending shell script code: # figure out the absolute path to the script being run a bit # non-obvious, the ${0%/*} pulls the path out of $0, c
by akamaka 12y ago
Here's the offending shell script code:
# 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 "${0%/*}" && echo $PWD)"
[...]
# Scary!
rm -rf "$STEAMROOT/"*
The programmer knew the danger and did nothing but write the "Scary!" comment. Sad, but all-too-familiar.
- ghuntley 12y agoOffending commit @ https://github.com/lrusak/steam_latest/commit/21cc14158c171f5912b04b83abf41205eb804b31#diff-9f48ba7f3b9e90e945e5b1fe7654c74cR358 https://github.com/lrusak/steam_latest/commit/21cc14158c171f...
- Qantourisc 12y agoWhenever I write something scary like that, I usually wrap it in a function, double checking if it's really the folder you wish to delete ... Often even adding a user-verification if possible.
- iamtew 12y agoSince the script is written in bash they could also have used $BASH_SOURCE, which will always point to the correct path of the script being executed, so you can do something like this: SCRIPT="$BASH_SOURCE" SCRIPT_DIR="$(dirname "$BASH_SOURCE")"
- deeviant 12y agoI worked at a solar SCADA company that rolled there own APT packages. In the pre and post install deb package scripts there was all kinds of crazy shit, like upgrading grub to grub 2 and manually messing with boot sectors. All this stuff in packages innocuously named modbus_driver.deb or what have you, and all in absolutely the most archaic bash syntax possible. Eventually somebody mixed a rm -rf /bin/* with rm -rf / bin/*, and the rest is history. They bricked about 100 embedded PC's, all in remote locations, all powering powerplant SCADA systems that did stuff like connect to CalISO for grid management or collect billing information. It cost hundreds of thousand of dollars to fix.
- deeviant 12y agoI worked at a solar SCADA company that rolled there own APT packages. In the pre and post install deb package scripts there was all kinds of crazy shit, like upgrading grub to grub 2 and manually messing with boot sectors. All this stuff in packages innocuously named modbus_driver.deb or what have you, and all in absolutely the most archaic bash syntax possible. I did suggest, strongly to jail all application binarys that we twiddle around with, with something like chroot, but was rebuffed. Eventually somebody mixed a rm -rf /bin/* with rm -rf / bin/*, and the rest is history. They bricked about 100 embedded PC's, all in remote locations, all powering powerplant SCADA systems that did stuff like connect to CalISO for grid management or collect billing information. It cost hundreds of thousand of dollars to fix.
- thaumaturgy 12y agoYikes. That should never have passed a code review. I know mistakes happen, I often defend screw-ups, but anything in a shell script that has an "rm" command should automatically get the squinty eyes.
- whoisthemachine 12y agoAlso, someone should have been asking "Why is that scary?" - which should have lead to a discussion on alternatives.
- userbinator 12y agoWould this have stopped things getting deleted? if [ -z "$STEAMROOT" ] # something isnt right...
- Alupis 12y ago> Would this have stopped things getting deleted? No. "${STEAMROOT}" will contain something when the "rm" command runs. It just might not be what's expected. > STEAMROOT="$(cd "${0%/*}" && echo $PWD)"
- colanderman 12y agoNot if the previous line was: STEAMROOT=$SOME_OTHER_UNSET_VARIABLE/ "rm -r " is a code smell, as much as "cc -o myprog .c" is. You should always know what files make up your system, and track them in a MANIFEST file. There's rarely a good reason to use wildcards when a program is dealing with its own files. xargs rm -df -- < MANIFEST fixes this.
- falcolas 12y agorm --preserve-root isn't a bad thing to have either. That way, even if you do screw up, you won't be able to run rm against '/', even with '-f'. It's one of the top aliases in my .bash_aliases file.
- colanderman 12y agoAgreed. Thankfully this is the default on many systems (I'm guessing recent coreutils). Unfortunately dumb lines like "rm -rf $HOME/$STEAMDIR" still get through unscathed.
- philh 12y agoI think that rm --preserve-root -rf /* doesn't save you.
- colanderman 12y agoYou are quite correct.
- enneff 12y agoGotta question why they used -f.
- frou_dh 12y agoIf all one is in the habit of using with a particular command is some single letter flags glommed together like that then it's possible to forget that they're actually separate flags.
- FranOntanaya 12y agoProbably to delete subfolders.
- unwind 12y agoThat's normal, but why the trailing slash?! That's just pointless, and almost looks like an explicit deathtrap. Without the slash, an empty variable would result in a command line of "rf -rf" which would simply fail due to the missing argument. There is absolutely no need for having a trailing slash, it's not as if "rf -rf foo" and "rm -rf foo/" can ever mean two different things, there can be only one "foo" in the file system after all. Very interesting way of introducing an epic fail with a single character, that really looks harmless.
- riquito 12y ago> it's not as if "rf -rf foo" and "rm -rf foo/" can ever mean two different things That's true, but the original code was akin "rm -rf foo/*" and that's different, since it removes the content of the directory while preserving it.
- teddyh 12y agoI have observed that ,any people feel that directories “need” to have a slash appended to them. They are afraid of ever doing, e.g. “cd /foo/bar” and will always do “cd /foo/bar/”. I”m guessing they feel like it would be some sort of type error, like treating a directory like a file or something. This behavior is especially common with regards to URLs; there are many flamewars about whether URLs “need” a trailing slash or not.
- EC1 12y agoI thought I was a bad programmer until I saw this thread. How did this make it to prod?
- greenknight 12y agoTight deadlines, limited resources (programmers), limited budget, limited QA