Stories
Slash Boxes
Comments

SoylentNews is people

posted by LaminatorX on Friday January 16 2015, @01:55PM   Printer-friendly
from the no-shortcuts dept.

If you're a Steam user — beware, even slight modifications of your system may result in the nuking of your home directory, and more!

Fortunately, as the entry point for the user is a shell script (bash, but that's another story), it's been quite easy to find the source of the problem, the lack of sanitising shell variables before passing them to potentially dangerous commands — in this case, “rm -rf "$STEAMROOT/"*'”. The commit that introduced the bug also seems to have contained a remarkably apt comment ``#Scary!'' (it's not clear that the repo being pointed to, and its commits, mirror exactly the same commits as Steam themselves would have added them.)

It seems that even on MS Windows, Steam gets a bit over-eager about deleting files it doesn't own.

As a software engineer, who's also been a package maintainer on huge projects with up to 70 engineers wanting to force patches into my tree, I've become hyper-attuned to the concept of asking "what could possibly go wrong" (and having a mindset like Bob the Bastard from the animated Dilbert series), and consequently for demanding small readable patches which do just one small thing that's trivial to review. Would the patch have passed review? How confident are you about the quality of the rest of the code if things like this can slip through?

Related Stories

Steam's Delete-All-Files Bug Has Been Patched 28 comments

Valve's changelog for the Steam client dated January 19 includes the fix for the Linux-related screwup.

Silviu Stahie notes at Softpedia

it might have deserved more than just an entry in a changelog, but we'll have to settle with just that.

"Fixed a rare bug where Steam could delete user files when failing to start"

Related:
rm -rf / Considered Harmful, to Steam Users

This discussion has been archived. No new comments can be posted.
Display Options Threshold/Breakthrough Mark All as Read Mark All as Unread
The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
  • (Score: 1) by WizardFusion on Friday January 16 2015, @02:04PM

    by WizardFusion (498) on Friday January 16 2015, @02:04PM (#135365) Journal

    This only happens when you uninstall Steam.
    I can see the reasons behind cleaning up the application folder when it is being uninstalled, but more validation is required.

    • (Score: 2) by Marand on Saturday January 17 2015, @01:37AM

      by Marand (1081) on Saturday January 17 2015, @01:37AM (#135573) Journal

      This only happens when you uninstall Steam.

      Nope, it happens when the reset_steam function is called, which occurs in one of two cases. One is if you explicitly call --reset, the other is inside an if statement near the end of the script that appears to be part of a version check. Having your update checker potentially attempting rm -rf /* is pretty bad.

      In either case, it calls reset_steam, which is supposed to move some files to a safe location, wipe the directory, restore the saved files, and start over. The problem is the script assumes that the variable gets set as expected, with no checks at all, and then runs risky, poorly-written commands based on that assumption. It looks like a lot could go wrong with the way they set this up, so it's a bit worse than "just don't uninstall/reinstall and you're safe".

  • (Score: 0) by Anonymous Coward on Friday January 16 2015, @02:15PM

    by Anonymous Coward on Friday January 16 2015, @02:15PM (#135367)

    Principle of least privilege for the win. My games account can't delete anything but itself, and it's all Steam.

  • (Score: 2) by arashi no garou on Friday January 16 2015, @02:19PM

    by arashi no garou (2796) on Friday January 16 2015, @02:19PM (#135368)

    Okay, I'm no programmer, and I just barely understand bash scripting enough to automate a few minor tasks on my machines. But even I know better than to pull something like this! How in the hell did this get released?

    • (Score: 2) by Sir Finkus on Friday January 16 2015, @02:26PM

      by Sir Finkus (192) on Friday January 16 2015, @02:26PM (#135370) Journal

      Probably a deadline that needed to be met, there was even a comment on the LOC in question that said it was scary.

      The correct way to do this is probably keep a running tab on all files that steam creates, then iterate through that list if you need to delete steam rather than deleting the steam directory. I don't know how well this would work with things like game saves and mods though. If it were me, I'd just pop up a window at the end of the uninstallation if the steam folder still had stuff inside of it advising the user that they can delete the folder to complete the uninstall after checking the contents to make sure there's nothing important.

      • (Score: 0) by Anonymous Coward on Friday January 16 2015, @03:30PM

        by Anonymous Coward on Friday January 16 2015, @03:30PM (#135386)

        No, the best way to handle a full removal is simply to not use environment variables. Use absolute paths only.

        • (Score: 2) by Geotti on Friday January 16 2015, @04:00PM

          by Geotti (1146) on Friday January 16 2015, @04:00PM (#135393) Journal

          The comment should have read something along the lines of "make sure $STEAMROOT is ALWAYS, (always, always, always, ...) set"

          • (Score: 2) by el_oscuro on Saturday January 17 2015, @01:36AM

            by el_oscuro (1711) on Saturday January 17 2015, @01:36AM (#135572)

            #!/bin/bash
            # Make sure it is defined
            if [ "$STEAMROOT" == "" ]; then
                echo $0: STEAMROOT not defined. Unable to cleanup steam 1>&2
                exit 1
            fi
            # Not a directory
            if [ ! -d "$STEAMROOT" ]; then
                echo $0: STEAMROOT \"$STEAMROOT\" is not a directory. Unable to cleanup steam 1>&2
                exit 1
            fi
            # Finally, we can run the cleanup. In the event STEAMROOT is pointed to a directory it shouldn't be,
            # include a hard coded subdirectory to it

            rm -rf $STEAMROOT/steam_files/*

            --
            SoylentNews is Bacon! [nueskes.com]
            • (Score: 2) by el_oscuro on Saturday January 17 2015, @01:40AM

              by el_oscuro (1711) on Saturday January 17 2015, @01:40AM (#135575)

              Actually the delete should have just been (no *)

              rm -rf $STEAMROOT/steam_files/

              --
              SoylentNews is Bacon! [nueskes.com]
            • (Score: 2) by hash14 on Saturday January 17 2015, @09:45PM

              by hash14 (1102) on Saturday January 17 2015, @09:45PM (#135729)

              Even easier if you're using bash:

              set -u

              Causes any attempt to dereference an unset variable to exit the entire script immediately.

              Bash, as a scripting language, is quite liberal and dangerous. It allows you to do crazy things (technically, any other language allows you to do stuff like this as well, but bash makes it painfully easy). A good practice in any bash script-writing is to peruse these options [tldp.org] and use them to make processing as strict as possible, just like any Perl programmer would start their scripts with

              use strict;
              use warnings;

              and perhaps some others. I have barely written more than 100 lines of perl in my life, maybe someone else knows more on this than I.

            • (Score: 2) by Geotti on Sunday January 18 2015, @06:11AM

              by Geotti (1146) on Sunday January 18 2015, @06:11AM (#135765) Journal

              I stand corrected. ;)

    • (Score: 2) by gman003 on Friday January 16 2015, @03:07PM

      by gman003 (4155) on Friday January 16 2015, @03:07PM (#135377)

      Valve has never been the best programmers, and Linux is a new platform for them. They might just not know better.

    • (Score: 1) by Refugee from beyond on Friday January 16 2015, @04:38PM

      by Refugee from beyond (2699) on Friday January 16 2015, @04:38PM (#135404)

      (ba)sh is a “gotcha” language. Too easy to screw up. At least that was my impression.

      --
      Instantly better soylentnews: replace background on article and comment titles with #973131.
      • (Score: 2) by Thexalon on Friday January 16 2015, @05:19PM

        by Thexalon (636) on Friday January 16 2015, @05:19PM (#135415)

        Bash certainly can cause trouble if you're acting as user "root" and relying on environment variables to specify the beginnings of directories. Believe me, I've FUBARed enough personal boxen to know exactly how bad it can get.

        That's why you shouldn't rely on external vars to be set, but actually, I dunno, check first before potentially deleting everything in the root directory.

        --
        The only thing that stops a bad guy with a compiler is a good guy with a compiler.
        • (Score: 0) by Anonymous Coward on Friday January 23 2015, @05:06PM

          by Anonymous Coward on Friday January 23 2015, @05:06PM (#137317)

          I was with you until you used the word boxen. Please don't.

          Go ahead and try to look it up, the only definitions (that dont relate to boxwood) are user submitted. At best it is outdated jargon.

  • (Score: 2) by tynin on Friday January 16 2015, @02:52PM

    by tynin (2013) on Friday January 16 2015, @02:52PM (#135374) Journal

    We had something similarly bad occur at work. Without going into too many details, a platform developer decided that he didn't want to trust operations to make sure file ownerships were set correctly. So a chown -R was put in to "correctly" set the ownership on the instance directory. Unfortunately, some of our users have their instance directory set to /, which likely wasn't the best place for it, but in their defense it worked fine for years prior. So when we upgraded to this newer platform build, suddenly some servers failed in interesting ways, and continued to fail after a reboot. Turns out some system dirs aren't very happy having their ownership get changed to a non-privileged user.

  • (Score: 2) by nitehawk214 on Friday January 16 2015, @03:08PM

    by nitehawk214 (1304) on Friday January 16 2015, @03:08PM (#135378)

    Eve Online managed to do this to windows users once. It overwrote boot.img or some root directory important file during install. I think it only affected certain versions, and I was not one of them.

    --
    "Don't you ever miss the days when you used to be nostalgic?" -Loiosh
    • (Score: 1) by ThG on Friday January 16 2015, @03:43PM

      by ThG (4568) on Friday January 16 2015, @03:43PM (#135389)

      Boot.ini was killed by the classic to premium graphics update (Trinity release).

      If that hadn't happened, I'd never have gotten my hands on the boots.ini (wearable apparel for EVE avatars)!

      • (Score: 2) by nitehawk214 on Friday January 16 2015, @04:19PM

        by nitehawk214 (1304) on Friday January 16 2015, @04:19PM (#135397)

        Ahh that was it. I was running either a too old or too new version of windows to be affected by it.

        Their big problem was they had a file called boot.ini in their installation directory and this overwrote it. I think a quick windows "repair" fixed it... as long as you have the installation media. :)

        --
        "Don't you ever miss the days when you used to be nostalgic?" -Loiosh
  • (Score: 1, Funny) by Anonymous Coward on Friday January 16 2015, @03:26PM

    by Anonymous Coward on Friday January 16 2015, @03:26PM (#135383)

    You're been Steam cleaned!

  • (Score: 0) by Anonymous Coward on Friday January 16 2015, @05:09PM

    by Anonymous Coward on Friday January 16 2015, @05:09PM (#135412)

    This used to work
    http://dilbert.com/fast/2015-01-13/ [dilbert.com]
    (You could get a slimmed-down page.)

    As of that date, it redirects and it appears that you need to allow some scrips(s?) to run.
    No longer interested.

    -- gewg_

  • (Score: 0) by Anonymous Coward on Friday January 16 2015, @05:59PM

    by Anonymous Coward on Friday January 16 2015, @05:59PM (#135429)

    Digressing for a moment,
    I made a worse mistake than this once upon a time.....I made a batch file which was designed to be called with the right click menu recursively removing a selected directory and all its contents. The command in the batch was something like RD /S /Q [some-variable-for-target-path]

    I committed to an operation one day and then proceeded to focus on something else.....after about 20 seconds I noticed the command prompt window was continuing to work overtime and not stopping! I stopped the operation and went snooping at my filesystem.....The recursive delete operation was starting from C: drive root! .....my batch script had a flaw! .....I lost private documents, my music collection, my family photos, WINDOWS system folders were missing large chunks.....whatever was left in memory was keeping my desktop alive, but my next reboot was going to be the last one !!!!!!

    Regarding this badly scripted deletion routine on Linux,
    as a Windows user I find this mistake quite bizarre! I always thought that Linux scripting was much more robust and exacting than this!
    I am not a programmer by the way, but I do have an affinity for the art of it.

    This 'C' sourcecode will generate a unique timestamp like this example: 2015011704540053

    #include <stdio.h>
    #include <time.h>
     
    int
    main(int argc, char *argv[])
    {
      time_t t = time(NULL);
      struct tm *tm = localtime(&t);
      fprintf(
          stdout,
          "%04d%02d%02d%02d%02d%04d",
          tm->tm_year + 1900,
          tm->tm_mon + 1,
          tm->tm_mday,
          tm->tm_hour,
          tm->tm_min,
          tm->tm_sec
      );
     
      return 0;
    }

    At initial program setup When the "STEAM" directory was first created, I would have also created a unique timestamp file at root of "STEAM" directory named dont-delete-2015011704540053.txt . This file name would also be registered and set into the configuration/uninstaller files for this program.

    When the time came for the program to run commands to recursively delete its "STEAM" directory, it would firstly look for the name of the timestamp file in its configuration; it would then perform the deletion routine using an "if exist [path-to-timestamp-file] else" condition.

    The "else" part of this condition would pause execution and pop up a message box with useful information and manual troubleshooting steps for the user before proceeding further.

    As I said previously, I'm not even a programmer but I highly value robust conditional and interactive logic in user-space. I suppose the mistake I made deleting the contents of C: drive taught me a BIG LESSON which I will never forget.

    • (Score: 2) by urza9814 on Friday January 16 2015, @08:16PM

      by urza9814 (3954) on Friday January 16 2015, @08:16PM (#135504) Journal

      At initial program setup When the "STEAM" directory was first created, I would have also created a unique timestamp file at root of "STEAM" directory named dont-delete-2015011704540053.txt . This file name would also be registered and set into the configuration/uninstaller files for this program.

      If I understand what happened here, that wouldn't have helped at all. The problem is the steam directory was never set. So on install, it's not set, it writes that file to root. Then on uninstall, it checks root, finds that file, and happily proceeds to nuke your entire system. Might work if the directory was properly set during install and then unset somewhere since then though.

      as a Windows user I find this mistake quite bizarre! I always thought that Linux scripting was much more robust and exacting than this!

      In is, in the same way that C is more robust and exacting than a Windows batch script. But software does whatever the programmer tells it, and if he tells it to nuke your drive, you're screwed. There isn't a single programming language in the world that will protect against programmer errors. Saying you shouldn't use Bash because of this is like saying you shouldn't use SQL because of injection attacks.

      A lot of *nix server processed boot as root and then drop down into their own unprivileged user account. Might be nice for Steam to consider something similar, that way to uninstall you can just delete that user account and be done with it...

      • (Score: 0) by Anonymous Coward on Friday January 16 2015, @11:38PM

        by Anonymous Coward on Friday January 16 2015, @11:38PM (#135553)

        [I am the parent poster]

        ...If I understand what happened here, that wouldn't have helped at all [...] Then on uninstall, it checks root, finds that file, and happily proceeds to nuke your entire system. Might work if the directory was properly set during install and then unset somewhere since then though.

        I don't see a technical problem with what I previously described; the conclusions in your answer would only exist if the programmer in question is the type I would describe as an "I DON'T GIVE A FUCK" PIECE OF WORTHLESS SHIT. I really do not expect to see this type of programmer working for STEAM.

        So using your case, let's say the text file dont-delete-2015011704540053.txt was written to root at installation.
        When the time came to uninstall/delete the parent folder of the text file, the script logic would be something like this pseudo batch code:

        @ECHO OFF
        SETLOCAL
        SET path-of-text-file = blah
        SET parent-directory-of-text-file = blah
        SET directory-path-of-text-file = blah
        SET root = blah
        IF %path-of-text-file% EXISTS (
        IF %parent-directory-of-text-file% NEQ STEAM GOTO STOP_EXECUTION_AND_POP_UP_MESSAGE
        IF %directory-path-of-text-file% EQU %root% GOTO STOP_EXECUTION_AND_POP_UP_MESSAGE
        RD /S /Q %directory-path-of-text-file%
        GOTO EOF
        ) ELSE (
        goto STOP_EXECUTION_AND_POP_UP_MESSAGE
        )
        :STOP_EXECUTION_AND_POP_UP_MESSAGE
        ECHO The "STEAM" directory cannot be deleted. The directory path is resolving to this location:
        ECHO %parent-directory-of-text-file%
        ECHO This is wrong, the deletion script may have a bug !!!!!
        ECHO
        ECHO The directory we are looking for is named "STEAM" and it contains a text file named "dont-delete-2015011704540053.txt".
        ECHO Please navigate to the "STEAM" directory and delete it manually.
        ECHO ...
        PAUSE
        EOF

        As I said before, using "if else" conditional logic a normal programmer will not have a problem deleting the directory safely, and if the deletion fails then the user gets a verbose troubleshooting popup message. This programmer who wrote this deletion script looks to have less brains than me AND I AM NOT EVEN A PROGRAMMER!!

        • (Score: 0) by Anonymous Coward on Saturday January 17 2015, @08:55AM

          by Anonymous Coward on Saturday January 17 2015, @08:55AM (#135619)

          Jesus fuck. Fuck off

          • (Score: 0) by Anonymous Coward on Saturday January 17 2015, @03:30PM

            by Anonymous Coward on Saturday January 17 2015, @03:30PM (#135668)

            you fucking cunt!...You clicked reply just to say that terse shit !!
            I put effort into that post !! I stopped my day to compose all those words !!
            I would squeeze your neck E X T R A F U C K I N G H A R D if you stood in front of me COCKBREATH !!!

    • (Score: 0) by Anonymous Coward on Friday January 16 2015, @11:06PM

      by Anonymous Coward on Friday January 16 2015, @11:06PM (#135549)

      Should've used Linux!

      • (Score: 2) by aristarchus on Saturday January 17 2015, @04:11AM

        by aristarchus (2645) on Saturday January 17 2015, @04:11AM (#135592) Journal

        Should've used Linux!

        Hey! Those were exactly my thoughts as a Windows user! What are the odds? Amazing! But then, I have not been a Windows user since '95. Oh, and I don't even own a Television. Yes, I am the most interesting "that guy" in the universe! (wait! Windows can do recursive directory operations! Wow, this is one operating system that has really grown up.)

    • (Score: 2) by FatPhil on Saturday January 17 2015, @01:51PM

      by FatPhil (863) <reversethis-{if.fdsa} {ta} {tnelyos-cp}> on Saturday January 17 2015, @01:51PM (#135655) Homepage
      > I always thought that Linux scripting was much more robust

      The word you're looking for is powerful. And thus more dangerous if you're careless. There's a fitness function in Unix which quite happily kills of those who aren't strong enough to survive.
      --
      Great minds discuss ideas; average minds discuss events; small minds discuss people; the smallest discuss themselves