hacked with r57shell.php

[jonlink]

# set register globals off
php_value register_globals 0

# set allow_url_fopen off
php_value allow_url_fopen 0

# set magic_quotes_gpc off
php_value magic_quotes_gpc 0

# set magic_quotes_runtime off
php_value magic_quotes_runtime 0

are the basic things to turn off

as far as the security of the php files goes, they are pretty safe. for more info search "secure config" and look at the first post called "question about security of php files."

I'm not sure that there is a safer way to have a dynamic website, and honestly I wouldn't worry about it, so long as you are careful with variables set by users you'll be fine. There are lots of dynamic websites that are very secure, if you have questions about specific code post it somewhere and ask— folks are usually pretty happy to help patch up the holes. scrypte mentioned validating your $POST & $GET's that is a very good idea, always.

a good basic one is:

function validate($value){
	if(get_magic_quotes_gpc()) 	$value 	= stripslashes($value);
	if(!is_numeric($value)) 	$value 	= mysql_real_escape_string(strip_tags($value));
	return $value;
}

and you'd use it like so:

$id=validate($_GET['id']);

[MarkR]

Personally, I'd say, make sure allow_url_fopen is ON, but

Audit any require() or include() of a non-constant expression VERY CAREFULLY.

Almost any use of require() or include() on a non-constant expression could contain a compromise; I rarely use this in my applications (Exception: including all files in a directory).

Likewise, any use of eval() at all, should be audited with a fine-tooth comb. For preference, eval() should not be used. Its very existence in your application probably indicates a bug.

There is another thing which can be very dangerous and is often over looked:

• The "e" modifier on preg_replace expressions. At least one vulnerability I know of used this.

For preference, all regular expressions in preg_replace should be constant. Any which aren't should be audited very carefully.

Mark

[ClarkF1]

mysql_real_escape_string is only useful if you're using a database.

Personally I check using file_exists everytime.....

$id = $_GET['id'];

//YOU COULD ALSO STRIP OUT ANY DODGY STUFF AS PREVIOUSLY SUGGESTED BEFORE PROCEEDING

if (file_exists("./$id.php"))
 include('./$id.php');

[MarkR]

Using file_exists is not sufficient to prevent the above code being abused.

Almost every case of include() with a non-constant expression indicates poor practice and should be eliminated by refactoring the application.

Mark

[jonlink]

mysql_real_escape_string is only useful if you're using a database.

Yes, and no. This will strip out any html and that sort of thing, which can be useful. It is what I use, though I am using it for a DB and it was written specifically with that in mind.

I meant it more as an example of what to do, depending on the use things need to be customized. In the case of using that either I or bretticus first suggested I don't know of a need to do it at all, though it isn't a bad idea.

- The "e" modifier on preg_replace expressions. At least one vulnerability I know of used this.

MarkR, what is it so I can avoid it?

[MarkR]

jonlink wrote:

MarkR, what is it (the preg "e" modifier) so I can avoid it?

http://uk.php.net/manual/en/function.preg-replace.php

The "e" modifier interprets the replacement string as PHP code which is eval()ed for each replacement.

In the case where the replacement string is non-constant, it may be possible for a user manipulating input data to inject code to be executed- this will compromise the application.

Mark

[Piranha]

jonlink wrote:

Yes, and no. This will strip out any html and that sort of thing, which can be useful. It is what I use, though I am using it for a DB and it was written specifically with that in mind.

No, it will not strip out any html. If you want to change from HTML marks to code instead you should use [man]http://www.php.net/manual/sv/function.htmlspecialchars.php[/man] since it really does it possible to run in HTML.

[ClarkF1]

jonlink wrote:

Yes, and no.

You can only use mysql_real_escape_string if you have already connected to a mysql database, that's what I meant.

If you're writing a script that doesn't require database access you can't use it to escape characters, you'd need addslashes and/or other functions.

[jigga]

Alright lots of good info from everyone, thank you for replying you guys. For right now I will be using the code Bretticus provided which was:

<?php

$array['users'] = '/viewusers.php';
$array['home'] = '/gohome.php';
$array['update'] = '/updatethings.php';
$array['delete'] = '/deletestuff.php';

if ( array_key_exists($_GET['id']),$array) ) {
    include($array[$_GET['id']]);
} else {
    include('index.php');
}

?> 

Altho he did mention this is just quick fix, so im guessing this is still somewhat vulnerable? What can I do to make it even more safe? Brtticus mentioned he discourages dynamic include paths, how do I go about not making it dynamic?

These might be stupid questions but I am what you might call a PHP newbie and the newbiest of the newbies at that so please help me out, as I mentioned the person who made the site for me I dont keep contact with anymore and now that I found out its vulnerable I have to try fix it all on my own and I am just in the learning process.

Thank you again guys.

[bretticus]

jigga wrote:

Alright lots of good info from everyone

Yeah, you got alot of good comments. I certainly recommend jonlinks php.ini settings to tighten your site up:

# set register globals off
php_value register_globals 0

set allow_url_fopen off

php_value allow_url_fopen 0

set magic_quotes_gpc off

php_value magic_quotes_gpc 0

set magic_quotes_runtime off

php_value magic_quotes_runtime 0

And yes, as someone mentioned, sometimes, of course, you need to set variables externally. As that person mentioned, you must at that point, filter the users input (make sure they send what is expected.) For example, in http://www.google.com/search?q=cross+site+scripting, google will want to make sure that a remote user cannot insert SQL code to be executed at the Database server (this is all hypothetical of course as Google probably has various layers that queries pass through.) In PHP, for example, something like http://www.google.com/search?q=;+DROP *; (maybe not a legitimate example, but should give you the idea that somebody might be able to modify your data for malicious purposes) could be really bad. So you'd want (again the example is hypothetical) to use a jonlinks validate function in this case.

As for your question, the way to take variables out of the include statement (dynamic paths) would either be just linking to individual pages or using GET variables to toggle content. Since the former is self-explanatory, I'll explain the latter only.

I have used a similar switch statement in the past to run various content output functions:

<?php
switch ($_GET['mode']) {
	case "users":
		showusers();
		break;
	case "delete":
		showdeleteusers();
		break;
	default:
		showsearch();
}

function showusers()
{
	//do something	
}

function showdeleteusers()
{
	//do something	
}

function showsearch()
{
	//do something	
}
?> 

As you see, all of my functions are in-line in the very same script.

Now, for follow-up, since changes like this may be very abrupt, you should be safe with the code I gave you, or even the example of a bunch of if elseif statements previously demonstrated by a responder here. Someone mentioned using file_exists(), which is a good idea as I don't believe it works on external (remote) files. You can throw that into the mix.

As always, good luck.

[MarkR]

Using file_exists() is insufficient to validate, as there are some cases where a file exists but you still don't want the user to see it.

Basically a non-constant include() is usually wrong and should not normally be used - especially with anything which could possibly have come from the user.

As I said before:

php_value allow_url_fopen 0

Does not enhance security in any way and it should be left enabled.

Mark