views:

98

answers:

3

I am not a PHP developer but I'm assessing the security of a PHP5 application.

The author relied on extract($_POST) and extract($_GET) in some places, outside of functions.

My suggestion is to call extract($_POST, EXTR_PREFIX_ALL, 'form') and change the code accordingly, but his stance is that any variable is being redefined inside subsequent includes anyway.

I can easily change the superglobals by providing, for instance, _ENV=something inside the post values, but superglobals are arrays and I'm turning them into strings, I'm not sure it can have evil effects.

I could have a look at the several isset() uses and go backwards from there.. but I imagine there are attacks of this kind that don't require knowledge or divination of the source.

Is there some interesting variable to be set/changed, maybe in the innards of PHP?

Thanks

A: 

I'm not aware of any universal exploitability.

Anyway, it definitely is awfully bad practice.

What the script's author is saying is that the script's security relies on him not forgetting anything in the subsequent includes, which is horrible.

For strong general arguments against global extract()ing, see What is so wrong with extract()?

Pekka
Unfortunately, I don't have the authority to just say "this is bad", or I wouldn't have asked :-(
Marco Mariani
+4  A: 

A common name for the database connection is $db, but that would just blow up the system, you can overwrite the $_SESSION variable.

session_start();

$_SESSION['test'] ='test';
var_dump($_SESSION);
$vars = array("_SESSION" => 'awww');
extract($vars);
var_dump($_SESSION);

output

array(1) {
  ["test"]=>
  string(4) "test"
}
string(4) "awww"

Overwrite variables $idUser or other fun stuff, want to mess up the iterables? Pass array('i' => 5) to extract, there are all sorts of fun you can have depending on scope.

Edit:

I just thought of another, if the form is handeling file uploads, why not try and overwrite variables named $file, $fileName, $fileExtention and see if you can get it to read files outside your permission level.

Kristoffer S Hansen
So this *can* be done? Oh my God
Pekka
Like the other superglobals, _SESSION is an array. I can change it to a string, but any use of _SESSION from that point would just cause an error. Now I'm looking at specific variables, thanks.
Marco Mariani
+3  A: 

For assessing "might" try this:

File:htdocs/mix/extraction.php

<?php
extract($_GET);
var_dump($_SERVER);//after extract
?>

and call it like that:

http://localhost/mix/extraction.php?_SERVER=test

After the extract on my Xampp the output looks something like that:

string(4) "test"

If any one knows anything about your variable naming and you use extract on $_POST or $_GET globals, then you have a serious problem. With a bit of time and work it would be possible to find out some namings by try and error.

Without knowing your source an intruder could try to hijack any global variabl like $_SESSION (but here it will only take any effect if you do the session_start(); before the extract($_GET), $_COOKIE or $_SERVER and even set specific values for them like that:

//localhost/mix/extraction.php?_SERVER[HTTP_USER_AGENT]=Iphone

If you use extract like that:

extract($var,EXTR_SKIP);

extract($var,EXTR_PREFIX_SAME,'prefix');

extract($var,EXTR_PREFIX_ALL,'prefix');

then you will be perfectly safe.

ITroubs
At first I thought your answer was just restating what we already said. Then I tried setting specific array items like that, and Oh My God :)
Marco Mariani