Help trying to protect my site

PHP MySQL

[yowza]

I recently built my first data-driven site. Now I'm trying to make sure it is secure. I know there are no guarantees, but I would like to thwart unsophisticated attacks at least.

If you could direct me to any tutorials, I would appreciate it.

The average visitor is only given SELECT as a privelege. There is no sensitive information in the database; however, I don't want anyone to delete info or anything.

One problem is that when my url has a parameter such as .php?widgetid=122 - if you erase the 122 it spits out a SQL error.

How can I supress these errors?

Thanks

[Timotheos]

Here's the thread to read...
http://www.webmasterworld.com/forum88/924.htm [webmasterworld.com]

Jatar_k, Should this be in the library?

[jatar_k]

also take a look at PHP Error Control Operators [ca.php.net]

and good call Timotheos ;)

[yowza]

Timotheos- Great thread!

Thanks for the help guys.

Anything else I should do to secure my site?

[BlueSky]

I recommend you turn register globals off if they aren't already. This will force you to write better code and to limit which variables can be changed by outsiders and from where. Most importantly, check each variable before ever placing it into a SQL statement. If it's supposed to be say an integer, then test to be sure it is first. If the test fails, don't put it in the SQL statement but branch off and do something else instead.

It's a good idea to suppress errors to the screen but do that when you're ready to bring the site live. It's best to fix all SQL errors though rather than depending on suppression.

SQL errors = potential hacking victim.

[webadept]

One thing that crossed my mind when I read your post, is that you said "The average reader only has SELECT permisions"? Since the average "reader" is coming in as the default user for the server you are on e.g. "nobody" or "apache", how exactly did you attempt to set this?

If (gawd forbid) you put in a function to add each user to the MySQL database user area, with some set of permissions, this could be a very large hole area, and I would check that code very carefully... but really I would seriously go back in there and try to figure out why it was there at all.

Now, you probably did not do this, but it did cross my mind, and the first thing I learned when going through possible bug areas was "never over look the obvious" so I bring it up with that spirit in mind.

[yowza]

webadept: Nope, I didn't do that.

There are three levels to my site.

3= Hidden Content Management area protected by one .htaccess password - only site owner has access with SELECT, INSERT, UPDATE, DELETE privileges

2= Wholesale Area - Clients can only be added from level 3 by the site owner. Login script from Dreamweaver (I assume it is secure because I can't cause any SQL errors - anybody know about DW PHP Login Script security?) Difficult to be approved to access this area. Clients have SELECT, INSERT, UPDATE, DELETE privileges

1= Public Area - Visitors have access to same info as clients, except for two tables. SELECT privileges only.

[webadept]

Okay.. just to clarify here, and make sure (sorry, I'm 32 hours up and probably shouldn't even be let near a keyboard at this point).. the way you are doing this then, is in your version of :

$link = mysql_pconnect("localhost", $DBUSER, $PASSWD) or die("Could not connect: " . mysql_error());

The $DBUSER name is changing, according to level granted, and you have (what two?) three? db-accounts, each with separate grant permisions, and these change not on user name, but on level of access they have currently passed with their password?.. or are you basing this on user name and keeping track of their security level in a database table ...

This last is better,(only because they don't have to keep signing in to get to the next level), unless the security level value is being passed around with them by some means, such as a cookie, or even a hidden value.

Zend has such a system on their website, showing how to do this:
[zend.com...]

Which is all fine and dandy, but I can hack that in about 5 minutes without missing the rythm of petting my cat Eddie.

Okay,, again, I'm tired, but the problem here is, if I can figure out the string, and learn, either by seeing other users strings, or by checking my own values, that another level exists, and is logiclly tied (1, 2, 3, 4 etc), then I can force a level change enough times to get more information on the system internals.

The alternative to this is having the code query the access level on each page creation, or page request .. which is the opening for a DOS attack. (If you are on a MS SQL base, this becomes a serious problem).

This is where SESSIONS save you a great deal of grief, and headache, because with a SESSION, all I have is an ID, with is normally an MD5 string, and is meaningless to everything in the world, except the server, which uses that to tell who I am. From that string, if I can figure out anything about the code you are using, then I'm probably way to good to be bothering with hacking your website.

Anyway.. I don't know enough really about what you are doing, those are just some thoughts to consider. Here are a couple of links that might help you profile your code. If I'm sounding rough, it is only my condition, not yours which this is coming from, you obviously know enough to be conserned about it, to a level of actully asking, so this tells me you are more intellegent than most.

[onlamp.com...]

This one is pretty good really
[landsmanns.com...]

This is a good project to download and at least go over what they are doing, and how.
[sourceforge.net...]

[pfft.net...]

and just for the "obvious" thing, if you have any of these in your code, find alternatives
require()
include()
eval()
preg_replace() (with /e modifier)
exec()
passthru()
`` (backticks)
system()
popen()

Hope that helps you out,

Glenn Hefley

[yowza]

Below is what my code looks like. I assume that this provides some security with the magic_quotes. However, like I said, I can cause errors by changing parameters in the url. If I change widgetid=122 to widgetid= I get a SQL error. Also, if I change it to widgetid=\122, I get an error. I can cause errors in many ways. $connPublic is defined in an external file and included with require_once()

$colname_rsGetWidget = "1";
if (isset($_GET['widgetid'])) {
$colname_rsGetWidget = (get_magic_quotes_gpc())? $_GET['widgetid'] : addslashes($_GET['widgetid']);
}
mysql_select_db($database_connPublic, $connPublic);
$query_rsGetWidget = sprintf("SELECT * FROM widgets WHERE widgetid = %s ORDER BY widgetname ASC", $colname_rsGetWidget);
$rsGetWidget = mysql_query($query_rsGetWidget, $connPublic) or die(mysql_error());
$row_rsGetWidget = mysql_fetch_assoc($rsGetWidget);
$totalRows_rsGetWidget = mysql_num_rows($rsGetWidget);

I'm very new to this. Dreamweaver wrote most of the code. I had to change some things to get them working right. The above is what Dreamweaver writes. I assumed it would be secure because the program wrote it for me, but if I can cause SQL errors it can't be secure.

By the way, thanks for those links Webadept!

[Timotheos]

You can use the is_numeric function to check that your id is a number.

Better yet is to learn regular expressions to check it.
(I'm speaking to myself here cuz I don't know reg ex too well either).
Maybe we could flag down one of the gurus to help us out ;-)

[yowza]

I was just looking at is_numeric() when I decided to check back here. Since all of my queries are checking for numeric data, I think it should work. I'm getting ready to test it out in a little bit.

[yowza]

Instead of this:

$colname_rsGetWidget = "1";
if (isset($_GET['widgetid'])) {
$colname_rsGetWidget = (get_magic_quotes_gpc())? $_GET['widgetid'] : addslashes($_GET['widgetid']);
}

I used this:

$colname_rsGetWidget = "1";
if (is_numeric($_GET['widgetid'])) {
$colname_rsGetWidget = (get_magic_quotes_gpc())? $_GET['widgetid'] : addslashes($_GET['widgetid']);
}
else
{
header("HTTP/1.0 404 Not Found");
}

It seems like it is safer to me with the is_numeric function else redirect. Does anyone else have any thought on this?

[Timotheos]

Yes, you'll notice in the user notes in the manual that is_numeric is not a complete solution for your application since something like 1e10 is considered as numeric. I'm assuming that would still generate an SQL error.

I think the 404 is fine if you don't want to output a more specific user error.

$colname_rsGetWidget = "1";
if (is_numeric($_GET['widgetid'])) {
$colname_rsGetWidget = (get_magic_quotes_gpc())? $_GET['widgetid'] : addslashes($_GET['widgetid']);
}
else
{
echo "Go away! Ya bother me.";
exit;
}

[yowza]

Timotheos:

Actually, 1e10, or anything similar (ie: 23asldjfk\\--234), doesn't cause an error. All that happens is the page doesn't show any data from the database at all. This is ok with me as long as it is secure.

[ergophobe]

I blieve the following works in all cases. Since regular expressions tend to be processor intensive, this is probably faster.

function check_int($string) {
if ( is_numeric($string) && is_int($string + 0)){
return "<p>$string IS an integer</p>";
}
return "<p>$string is NOT integer</p>";
}

You must do both tests because

"12.3" is numeric, but not an integer

"123 " (space at the end) + 0 is an integer, but "123 " is not numeric.

It gives the correct result for

echo check_int ("123");
echo check_int ("1e3");
echo check_int ("12.3");
echo check_int ("123 ");
echo check_int ("12 3");
echo check_int ("123asd");

Unless you want to correct for typos by taking just the digits out of the string and treating that as an integer, you don't need regex. If you're worried about security, I would say you want a strict tolerance for typos and don't want to correct them on the fly. Why help the hacker who types "123asdf" by transforming this for him into a valid login (or whatever)?

Cheers,

Tom

[edited to improve code]