Bug 1202858 – Restarting squid results in deleting all files in hard-drive

https://bugzilla.redhat.com/show_bug.cgi?id=1202858

by geoffbp • 11 years ago

429 168 11 years ago

bugzilla.redhat.com

fermigier• 11 years ago

Scary. Even more scary is the fact that the bug has been open for a week, one person has confirmed 100% reproducibility, and no one seems to care at Red Hat. Isn't deleting peoples hard drives a big no no ?

cynix• 11 years ago

RedHat is pleased to announce the general availability of our new RedHat Enterprise Data Recovery service. Please contact your account manager for details.

TheDong5• 11 years ago

It already has a "fixed in version" listed. Fixing it within a day doesn't sound like no one caring :)

Does it really matter if no one commented "Oops, we screwed up"? It's kinda self-evident that there was a mistake and there's not really much to say; it's clear from the description how bad it is and marking it "Fixed in version" already says it all pretty much.

EdwardDiego• 11 years ago

Does it really matter if no one commented "Oops, we screwed up"?

To me it does. You just deleted someone's hard drive, an apology wouldn't be out of order.

raldi• 11 years ago

And this is why every bug tracking system should have a triage SLA.

Or, even if you don't want to declare a threshold, publish the current stats on its front page: "Over the last 30 days, our 99-percentile triage wait time was: XX hours."

Similarly, open tickets with priority=urgent should never go 24 hours without a new comment from the owner.

Animats• 11 years ago

"Or, even if you don't want to declare a threshold, publish the current stats on its front page: "Over the last 30 days, our 99-percentile triage wait time was: XX hours."

Now there's a good idea. Major open source projects should have software quality dashboards tracking things like that.

We're now seeing hospital emergency rooms displaying their current wait time in minutes on billboards.

oogway• 11 years ago

Well, the bug was reported against RHEL 6.7 which has not been released yet.

cssmoo3• 11 years ago

RH don't have a great reputation here. Unlike Debian which does proper triage and practices "zero release-critical bugs", RH threw out RHEL7 with loads of critical issues still open.

cellularmitosis2• 11 years ago

Ahh Redhat, the distro which chose to symlink a bunch of binary lib files from Apache into the etc directory.

"Why is grepping /etc taking so long? Binary files in /etc?!? WTF?!?"

Coming from Debian, Redhat seems to make a lot of irk-worthy choices.

Padding• 11 years ago

Is there an aricle with specifics on this and why it was done?

Erwin• 11 years ago

I find the /etc/httpd/logs symlink more annoying. If you want to grep through your Apache configuration you have to explicitly grep through conf and conf.d otherwise just going to /etc/httpd and doing a grep -r you're searching through gigs of Apache logs.

lmm2• 11 years ago

As opposed to Debian, the distro which chooses to break tomcat (a program which unzips into a single folder and is thereby completely self-contained) up into a million different pieces and scatter them randomly all over your hard drive?

rwmj4• 11 years ago

It's already fixed, plus there are many private comments which you cannot see (but I think you see "missing" comment numbers). Also the product in question is not released yet. It's good this was found, but no customer would have been affected unless they were using an alpha.

kevan3• 11 years ago

I love how deadpan the bug report is:

    Actual results:
    All files are deleted on the machine.

    Expected results:
    Squid is restarted.

Not many details yet but it sounds similar to the Steam bug [0] from last year.

[0] https://github.com/valvesoftware/steam-for-linux/issues/3671

viraptor3• 11 years ago

I would bet on the issue being in the init script itself rather than squid. (I'm assuming squid doesn't run as root by default in rhel) If that's true then it's another point for more sane process managers (upstart/supervisord/systemd/...)

kevan3• 11 years ago

Agreed, I should've elaborated. All it takes is something like this in the init script without checking if the variable is empty:

    rm -rf "$STEAMROOT/"*

SwellJoe• 11 years ago

This is, as the bug notes, a regression, and I'm guessing you're right about it being in the initscript (I'm pretty sure). I used to be a very heavy Squid user and Squid developer and I remember a very similar bug many years ago. It was in the cache_dir initialization code. It would read the configuration file, parse out the cache_dir lines, and if the directories didn't exist it would create them as part of the startup.

There were some circumstances where if there was no cache_dir line configured, or if the cache_dir was a link or something, the details are very sketchy in my mind after so much time, but it would end up destroying /.

I'm guessing this is of that same nature.

4ad2• 11 years ago

RHEL uses systemd.

blueskin_2• 11 years ago

"systemd" and "sane" only ever go in the same sentence as "sane people don't use systemd".

It looks like a bug in the init script; runnign it as squid's user wouldn't have triggered destroying the whole filesystem; likely just squid's config and anything under its /var.

stevenspasbo• 11 years ago

I started laughing at the lines

    Stopping squid: ................[  OK  ]
    rm: cannot remove `/boot': Device or resource busy

blcknight3• 11 years ago

Except the Valve bug passed their QE process if any and got out into the wild. This is for an unreleased version of RHEL, and was caught....

TazeTSchnitzel3• 11 years ago

QE? Is that a misspelling of QA, or is there another meaning beyond Quantitative Easing?

ams6110• 11 years ago

Note that:

  At the time of this writing, RHEL 6.7 is still pre-beta
  and this bug was found in an *UNRELEASED* update to squid.

Bad enough, but not like it's out in production.

elchief• 11 years ago

We had one of these kinda bugs.

If you uninstalled our software it deleted a major chunk of your Windows registry, crippling your computer. It was a one character error in our script. The first ticket read "Uninstalling [Product] destroys your computer".

I was responsible for customer support. Good times! Was a rough week. We managed to not get sued.

VieElm• 11 years ago

Pretty terrible that such a thing is even allowed by Windows. This is why as a user I like Apple's OS X sandbox.

sz4kerto• 11 years ago

If a software needs/has root rights, then all bets are off. This is true on any OS.

antientropic• 11 years ago

Similar things have happened on OS X: http://apple.slashdot.org/story/01/11/04/0412209/itunes-20-i...

jakobegger• 11 years ago

About a year ago or so I tried to fix a computer of an OS X user where the Dropbox installer somehow deleted the home directory and replaced it with the dropbox application... That was an odd experience (and one where sandbox didn't help...)

Navarr• 11 years ago

Windows 8 introduced a sandbox and users hated it. (Probably because it also forced digital signatures but hey, specifics)

akandiah• 11 years ago

It looks like the start-up/shutdown script is doing an rm -rf on a bash variable that's evaluating to null.

rosser• 11 years ago

If that is, in fact, the case, it is indeed a repeat of the Steam bug that 'kavan mentions elsethread.

smrtinsert2• 11 years ago

It's hard to trust anyone that would ever depend on such a variable without having some sort of precondition.

TheDong5• 11 years ago

"it's hard to trust anyone that writes a dumb bug".

Listen to yourself. We've all written dumb bugs. We've all had that one line of code that was an obvious mistake. I still trust people who write a bug here and there because if I didn't I would have to forgo trusting everyone for everyone makes dumb mistakes sometimes.

spacemanmatt• 11 years ago

Aren't you glad for QA? Humans are inconsistent.

TheDong5• 11 years ago

Citation needed.

It very well could have been an instance of the bumblebee bug (https://github.com/MrMEEE/bumblebee-Old-and-abbandoned/issue...) where it's "rm -f / some/file" instead of "rm -f /some/file"

Link to the code if you want to make that claim.

ffn• 11 years ago

Thanks OP, this actually made me laugh uproariously. Anyway, I'd be willing to bet 100 push-ups that (unless it was malicious and not a bug), this thing is caused by some clean up code somewhere that originally intended to do "rm -rf /path/to/squid/socket" but the function that was suppose to generate the "/path/to/squid/socket" string instead generated a null which was then parseString'd onto a "" via some + function that was trying to do "/" + null.

But I'm neither a redhat user nor an OS dev, so I might completely wrong.

profmonocle• 11 years ago

That's almost exactly how I blew up a test server once. (rsync --delete in place of rm) Taught me to be extremely careful when dealing with absolute directory paths.

marcosdumay3• 11 years ago

Oh man... Rsync deserves extreme caution even compared with other bash commands. It's so easy to erase everything and copy terabytes again.

mplewis• 11 years ago

I'm having trouble finding the related commit that fixed this, can anyone else find it?

rwmj4• 11 years ago

This bug is an internal QA bug. The reporter is a Red Hat tester. The buggy RPM was never released outside Red Hat, and so there's no requirement to release the source. When RHEL 6.7 comes out the source will go up on ftp.redhat.com.

int19h• 11 years ago

10 bucks says the lesson learned will be "remember kids, always set -u (and other good ideas, like set -e, set -o pipefail, and personally I like set -o posix but you'll need to give up process substitution for that)."

thaumaturgy3• 11 years ago

Hmm, there are two possible candidates in the init.d script for the RHEL 6 package of an older version of squid (doesn't look like bug submitter is using a current version).

In stop():

    rm -rf $SQUID_PIDFILE_DIR/*

and in restart():

    rm -rf $SQUID_PIDFILE_DIR/*

SQUID_PIDFILE_DIR is hardcoded to "/var/run/squid" at the top of my copy of the init script. But, neither of those rm commands check first to make sure that SQUID_PIDFILE_DIR isn't empty (or, better yet, is in /var and doesn't contain ".."), and either the submitter's copy of the script is mangled or something else somewhere is stomping on SQUID_PIDFILE_DIR in the shell environment.

...I should grep my init scripts for "rm".

rachelbythebay• 11 years ago

squid-3.1.10-29.el6.src.rpm (from ftp.redhat.com, buried where they keep SRPMs) has squid.init within, and that file has no mention of SQUID_PIDFILE_DIR. A few other spot-checked versions are the same way.

https://github.com/mozilla-services/squid-rpm/blob/master/SO... ... however ... whatever that is, does.

Did they take this upstream init script somehow?

ato• 11 years ago

This looks like it:

https://bugzilla.redhat.com/show_bug.cgi?id=1102343

I guess they applied that change which was obviously written against a very different init script where the variable is actually defined, got QA to test it and immediately backed it out.

thaumaturgy3• 11 years ago

That would be my guess too, although the package maintainer confirmed it on (presumably) a fresh install. I can't find a candidate rm command anywhere in the SRPM, so maybe an upstream file got merged into distrib somehow? I don't have access to an RHEL system to try it out, and can't find the distrib RPM yet to check.

jedisct12• 11 years ago

My good old trick to mitigate that is:

    touch /-@

I also always do it in my home directory:

    touch ~/-@

That's the first thing I do on a new host.

When accidentally running rm -f *, the command expands to -@ first, which is not a valid option and makes the command fail before doing any harm

    rm: illegal option -- @
    usage: rm [-f | -i] [-dPRrvW] file ...

lambeosaurus• 11 years ago

As someone who uses the commandline a lot but isn't exactly a wizard, why does this work?

lsb• 11 years ago

globbing expression expand to all the files that match.

if a directory has 1.txt, 2.txt, and 3.txt then << rm * >> expands to "rm 1.txt 2.txt 3.txt" and is then executed.

if you have -@, 1.txt, 2.txt, and 3.txt, that expands to << rm -@ 1.txt 2.txt 3.txt >>, and that can't execute.

(if you really wanted to remove your -@ you'd do << rm -- * >> because a double-dash signals the end of command-line options.)

ulrikrasmussen2• 11 years ago

The * is expanded by the shell to a space-delimited list of filenames, but the shell does not adequately escape filenames that can be misinterpreted as arguments to 'rm'.

raverbashing• 11 years ago

More specifically, it is expanded on the shell

Example:

$ echo /*

/bin /boot /dev /etc /home (...)

JoshTriplett• 11 years ago

That won't actually help with rm -rf /* , only with rm -rf * in / or $HOME.

hk__2• 11 years ago

Would it work with `touch "/ -@"` (not the space)?

ioquatix• 11 years ago

Or just use zsh.

michaelx386• 11 years ago

Would zsh still protect me if the script explicitly uses Bash (i.e. #!/bin/bash)? Sorry if this is a dumb question, I'm unsure how shells work when calling other kinds of shell scripts.

anh794• 11 years ago

zsh is an interactive shell. It is not to replace #bash as system shell as I know.

ulrikrasmussen2• 11 years ago

While this is a really nice hack, stuff like this is also the reason I feel really uneasy when writing shell scripts. What works now may suddenly break in the future due to inadequate escaping of filenames.

miles• 11 years ago

Neat idea! Just tested that in an OS X 10.8 virtual machine; while it works nicely against "rm -rf *", sadly it does not help stop an accidental sudo rm -rf / or ~/. Also, "touch ~-@" created a file in the home directory called "~-@"; in order to set the correct filename, I cd'd into ~ and then ran "touch ./-@".

amadvance• 11 years ago

Likely you mean: touch ~/-@

atmosx• 11 years ago

Hm that's a cool trick. IIRC some distributions (Suse?) had a 'bash' clause where you couldn't do "rm -rf /" without 'y'.

My 'zsh' has this one too, when I 'rm -rf /some/dir/' always asks if I'm "sure". Truth to be told, I'm not even expecting the text in "stdout" anymore, my finger goes to the 'y' automatically, which means that if I make something stupid it won't be able to protect me :-P

The last couple of years I stopped doing 'stupid things' by stop working on the shell when I'm very tired. That was the cause of my rm-related-incidents in the past :-)

iancarroll• 11 years ago

Some versions of RM (Ubuntu/Debian if I remember correctly) require --no-preserve-root.

Aissen• 11 years ago

rm -rf -- $EMPTY_VAR/*

Ooops.

liviu-• 11 years ago

  touch /--
  rm -rf *

(admittedly, this would be a malicious attempt rather than a careless bug)

Zenst• 11 years ago

Very handy trick. More so if no root access, myself I preer to rename rm and drop a shell wrapper in place and can be simply a case of changing all passed "" into "Sorry_Dave_I_can_not_allow_you_to_do_that" (or words to that effect) and all "-AsteRISKdeleteALL" into "-" and then that modified input is passed onto rm. But can adjust how and add rules to taste.

That way the pain of having to type AsteRISKdeleteALL instead of * for rm events offsets any anxiety by far.

You can also catch the rm and mv to a difectory with quota's you can call a recycle bin, some low end attached storage can be fine as well as not many situations when your wildcard deleting with a time factor. Can accommodate this in your own skulker to clean up in a more organised way overall in a timely manner. and and scripts you can path to the real rm command if needs be, last time I called it P45Generator, but not the finest for readability in any such scripts.

quoiquoi• 11 years ago

alias rm='trash-put'

anh794• 11 years ago

Maybe I would add a new record to this https://github.com/icy/bash-coding-style#good-lessons

chanux• 11 years ago

..aaand this bug happened in 2014 as well?

anh794• 11 years ago

no, it's my mistake. I have fixed that.

zx2c4• 11 years ago

    "Thanks Swapna and Red Hat QE for catching this issue before the package was released. Great work!"

Looks like this wasn't released into production.

stefanha• 11 years ago

This comment should be at the top. False alarm, squid users, and now back to our regularly scheduled HTTP proxying.

For more details on why this issue doesn't affect squid users: https://rwmj.wordpress.com/2015/03/24/restarting-squid-resul...

Bug 1202858 – Restarting squid results in deleting all files in hard-drive

Related Stories