Stories
Slash Boxes
Comments

SoylentNews is people

SoylentNews is powered by your submissions, so send in your scoop. Only 18 submissions in the queue.
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?

 
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: 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?

    Starting Score:    1  point
    Karma-Bonus Modifier   +1  

    Total Score:   2  
  • (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.