But of course, script kiddies aren't particularly good at securing the servers they pwned, are they? [1]

A couple of months ago, I wrote QADE [2] for fun, a "quick and dirty" PHP-based text editor and webshell for editing files and running commands remotely. I deliberately failed to implement any authentication-related feature, because I didn't want to give an illusion of security. A secure webshell is an oxymoron.

[1] https://blog.avast.com/2014/06/09/are-hackers-passwords-stro...

[2] https://github.com/kijin/qade

Not only is the idea completely nuts, but all Google produces for C99shell is how it's used in backdoor tools.

Surprises: Zero.

C99shell is easy to use and common.

Why is it nuts?? You're using it for quick and easy defacements and compromises.

This is a numbers game often from people with low skill levels. I see no issues here at all. If your defacements were getting quickly taken back by another crew it might be an issue, but I've seen no evidence of this happening.

Just because there's a security issue it's not the end of the world. Risk management still comes into it.

Easy to use = more defacements. Might have a hole, well is there evidence we are losing enough defacements to justify the retraining of the crew and added costs?

And more importantly, be agile. Defacements getting retaken over at unacceptable levels, just move to a different product.

Uploading PHP files with system() calls works too, but C99 is much easier to use.

But yes, I only came to know it when I was a stupid defacer teen. Shame on me.

(Answer: yes)

What's really bad about extract() is that the default behavior is to overwrite existing variables. This is just a recipe for disaster, no less than the infamous register_globals.

I once wrote a framework where the controller would pass variables to the view as $view->currentUser and the template would access it as $currentUser instead of the more verbose $this->currentUser. This was implemented with extract(), but I went to great lengths to make sure that the scope was clean.

PHP probably ain't gonna get rid of extract() any time soon, but at least they should change the default to EXTR_SKIP.

    function qux() {
      return compact('foo', 'bar', 'baz');
    }

    extract(qux());

Obviously you can do the same thing by just passing arrays but sometimes a simple variable is easier and less cumbersome.

Wow. This is the anti-Scheme. It's like they took everything good in language design and decided to do the exact opposite. "Let's make a function that has the side effect of introducing variables into scope. That's a great idea."

Oh, Bog....

Since when is Scheme the arbiter of what is good or bad?

PHP lets you do this, or not do this, your choice. Like C, PHP doesn't tell you what to do, it's up to you.

That's why people actually use it.

Language hacks like the one we're discussing are fine IF:

- You work with people who avoid bad features.

- You can avoid using code that uses bad features (less of a problem, until you have to debug things and then you're in a world of hurt).

- The features are not short-sighted hacks that prevent the language from moving forward (e.g., eliminate the opportunity to make things faster through dynamic compilation or whatever).

Scheme is a great language that is not useful in the real world, while PHP is a terrible language that happens to be in wide use. Neither of these positions are unique, and honestly I'd much rather use a bad language with good tooling than a great language with poor support. But I will continue to point out PHP's flaws, which are many and just howling bad, and work towards improving the alternatives.

Your post might be [slightly] humorous, but as an argument against PHP language constructs it fails miserably.

Features that have limited real benefits with lots of risk get abused all the time. They are the retarded tools of the world creating technical debt for everybody else and they should be retired.

Extract is one of the stupidist language feature conceived precisely because it puts something so ripe for abuse into the hands of idiots. It even has a simple name that practically encourages its abuse.

Or making critical parts of infrastructure work by a cron job that calls wget.

For some reason PHP needs extract, because a dictionary just won't do.

  foo["var_x"]

  // for some reason needs to be:

  $var_x

My point is that trusting user input is the error, not having the ability to play with the scope.

    $login = '1234';
    extract(array('login'=>true, 'messsage'=>'hacker'), 'message');
    var_dump($login_, $message);
    // => $login='1234', $message='hacker'

You can set the EXTR_SKIP flag to do something like that. There are a bunch of other flags as well to control how it works.

    function qux() { return [$foo, $bar, $baz]; }

    list($foo, $bar, $baz) = qux();

Coffeescript has a really neat destructuring operator for this kind of thing:

    qux = () -> {blah: "a", blorp: "b", zoop: "c"}
    {blah, blorp, zoop} = quz()

It's also error prone if you have several feeder/user functions since you have to change them in multiple places.

Or if you have two code paths that end up with two different sets of variables (probably controlled by a semaphore variable) then this won't work.

Ghetto templating (using a PHP file as a template). Extract an object from the database into local vars to echo in the "template".

Parsing fixed width files, zip the columns with their names, then extract in the processing function.

In both cases you know exactly which vars are being replaced. The real WTF is extract on $_{REQUEST,GET,POST,SERVER,...}.

Encrypted and signed files sent over the wire from a company we do business with. Additionally I was just pairing the values up with local names that I chose (the fixed width file had no column names itself, they sent us a word doc (ugh)). And the function that called extract had exactly one local, the array I built representing a row from the file (unused after the initial call to extract).

Extract also lets you prefix the extracted vars, avoid overwriting name collisions, etc. http://php.net/extract

The code from this article is unsafe because it directly operates on user input, was not explicit about what values were required (you can filter an array by key easily enough...) and doesn't isolate the environment it's extracting in. That's the unsafe behavior.

    function foo($option1=1, $option2=2, $option3=3, $option4=4) {
        //do stuff with params
    }

    foo(null, null, null, 10)

    function foo($options) {
        $option1 = 1;
        $option2 = 2;
        $option3 = 3;
        $option4 = 4;

        extract($options);

        //do stuff with params
    }

    foo(array('option4'=>10));

I personally like to work with explicit variables within the function rather then array indices. With your system I could easily call extract on your resulting array, but I also like defining the variables I want overridden explicitly in code so I my IDE doesn't think they are undefined (though JetBrains does a pretty good job of determining which variables are being created by extract in most scenarios anyway).

The security problem lies in the fact that the authentication code is conditionally called depending on user input.

    if ($login) { … }

    if ($_POST['c99shcook']['login']) { … }

It was not meant to be user input

Imagine this setup:

1. you have Nginx at $DOMAIN, with your well-oiled API service (with no HTML rendering logic to be found within it) mounted to /, and a PHP FCGI instance sitting on /views.

2. Nginx is configured such that /views is only accessible from localhost.

3. Your webapp on / mostly sees requests with "Accept: application/json" or some such. It responds to these directly.

4. If your webapp sees "Accept: text/html", it does the same stuff it would to render a text/json response... but then takes that response body, and sends it in an HTTP subrequest back to Nginx, to, say, /views/accounts.php.

5. PHP does this:

  extract(json_decode($_POST['response']));

6. Your webapp catches the response from Nginx, and sends it back without modification to the client.

It would be cool if somebody wrote an automated script which would seek out these c99 and try to identify those which are used on hacked sites. It could then use this to get access and remove this script and fix the original exploit.

Using exploits to help people is of course a can or worms but I like the idea of good hackers helping everyone.

http://www.securityfocus.com/news/203 http://en.wikipedia.org/wiki/Max_Butler#FBI_investigation.2C...

That said, I would agree completely that it's very legally dangerous. If it's not yours, don't mess with it!

As an academic matter, I think such a worm could end up being socially useful, if there are enough compromised machines and the people running them are sufficiently incompetent and those machines are being used against other people and you can be sure that your fix doesn't break something else and the machines just won't get re-compromised again next week. That's too many conditional clauses for me, but maybe someone else feels like taking one for the team.

Legally: again, don't do it. It's not yours.

Example: https://github.com/search?l=PHP&p=1&q=extract%28%24_GET%29%3...

Yuck...

http://webcache.googleusercontent.com/search?q=cache:http://...

No.

This is due to insane usage of the extract() function. Not a vulnerability with the function itself.

You can pass user-supplied input directly to plenty of other functions which have equally idiotic outcomes, it doesn't mean that they have vulnerabilities, it means the author is a liability.

    extract($_GET, EXTR_PREFIX_ALL|EXTR_REFS, 'gVar');
    extract($_POST, EXTR_PREFIX_ALL|EXTR_REFS, 'pVar');
    extract($_COOKIE, EXTR_PREFIX_ALL|EXTR_REFS, 'cVar');

http://www.yyy.com/script.php?hello=world Results with: $gVar_hello being the variables holding 'world'.

Is this poor form?

I previously used: `import_request_variables` - but thats been sidelined.

Not that it is a massive deal... I guess I just got in the habit when I was younger. Like I said, I always used import_request_variables (with various prefixes).

----

But back to my question - is it bad to use like I have?

  echo $blah; // hi <script>alert('foo');</script>

Second: it will double the memory used.

Third: you can't use the variables global anymore

Good point on the memory, but I wouldn't think thats a big issue. I haven't tested right now, but I dont remember ever having issues using the $_GET variable after exporting? Not sure if thats what you meant.

I can't immediately think of a practical way to make a problem out of that. But, you're making a couple of bets here: you're betting that there never will be a problem with it, and you're betting that the rules in PHP won't change in the future. All those folks that relied on magic_quotes already got boned by that second bet.

So, no, I wouldn't do it that way, but I wouldn't criticize you for it either.

  $blah = "hi $_GET[hello]";

How is this even news?

As far as I'm concerned though, Google and Firefox's malware checker engines should blacklist any domain that has the c99.php file located on it and block their webbrowsers from connecting to it in the first place.

Of course - correct me if I'm wrong here.

https://www.google.com/search?client=ubuntu&channel=fs&q=all...

    grep -Ri c99 /path/to/htdocs 

There are a few tools floating about that try to use a more signature-based approach to searching, and clamav has some signatures for the shells, but they can be hit-and-miss, as the obfuscation often changes.