On Tue, January 30, 2007 4:14 pm, nitrox . wrote:
> Richard, ive included your suggested code and now my php script is
> working
> properly. But I dont want to be a php copy/paste newb who has no clue
> of how
> things are working. If its not too much would you (or anybody) give a
> brief
> explanation of what this code is doing? Or are there any tutorials
> online
> that I can read that will educate me on this? Thanks again to all for
> your
> replies. Ive saved them all for future reference.
>
> atleast this part: $user_id = mysql_real_escape_string((int)
> $_GET['user_id']);

$_GET is an array which has all the values from the URL.

For example, this URL:
http://example.com/yourscript.php?user_id=42
would cause PHP to pre-build this for you:
$_GET['user_id'] = 42;

If you didn't care about SECURITY, you could just do:
$user_id = $_GET['user_id'];
and go on your merry way.

All the rest of the stuff is about SECURITY.

Bad Guys know that you are probably using that user_id in an SQL
query, and will do mean things like:
http://example.com/yourscript.php?user_id=NULL
http://example.com/yourscript.php?user_id=
http://example.com/yourscript.php?user_id[]=
http://example.com/yourscript.php?user_id=1
http://example.com/yourscript.php?user_id=reallylongstringtocauseabufferoverflowfollowedbynastyexecutablecode
http://example.com/yourscript.php?user_id=42 OR user_id IN (update
mysql.users  set password = password('password') where user = 'root')
.
.
.
in attempts to break into your website/webserver or otherwise wreak
havoc.

I did not URL encode the value in that last URL for demonstrative
purposes.  Plus most browsers fix that for you these days anyway.  And
I don't know if any SQL engine would actually *DO* that, but it's sort
of the idea of much more complicated things that could work.

So, to tear apart the rest.

(int) in PHP will type-cast a value to an integer.  This means that if
you do:
$user_id = (int) $_GET['user_id'];
you are immediately protected from most of the hacks above, since you
are forcing the $user_id variable to contain an INTEGER, such as 42,
0, 27, or -14.

You have thrown out the possibility that they are going to splice in
some nasty text SQL.

It's not perfect, as they could still perhaps manage to pretend to be
user #5 instead of user #42, if your script was silly enough to
implicitly trust GET input data.

The mysql_real_escape_string() part is a function provided by MySQL
which will make sure that the value you are providing is properly
escaped.  It is a more mature solution to what
http://php.net/addslashes and Magic Quotes were trying to solve.

Specifically, suppose your user_id in the database was NOT a number,
but was just people's names:
'Richard Lynch'
'Nitrox'
'Kevin O'Brien'

Ooops.  Look at that last name.  The ' in O'Brien is going to confuse
the ' that MySQL uses to delimit a string.

mysql_real_escape_string() takes the 'Kevin O'Brien' and converts it
to 'Kevin O\'Brien' so that when the MySQL SQL parser reads it, it
*knows* that the \' in O'Brien is part of the name.

That way, MySQL can store the right thing down in the guts of its
storage: Kevin O'Brien

Calling mysql_real_escape_string() on an integer is probably a bit of
overkill -- There is NO WAY for anything other than [0-9-] to be in
the result of the (int) typecast.

However, recent attacks have been using very funky features of Unicode
character sets, for languages other than English, so that what looks
like a perfectly normal valid input in English turns into something
realy nasty (in computer science terms, not linguistics).

I don't *think* there are any character sets for which 0-9 and - are
anything other than 0-9 and -, but I'd rather be safe than sorry.

To understand more about why I added all this stuff, start reading here:
http://phpsec.org
and keep reading that site until it's second nature.

>               die ("Could not perform query $query: ".mysql_error()."\n");

This is a Bad Thing to do on most newbie servers where php.ini has:
display_errors = On
because it will expose your exact query and all kinds of mysql stuff
to the Bad Guys, which just makes it easier for them to attack.

You should explore setting up your server with php.ini or .htaccess so
that you have display_errors OFF and log_errors ON, and then you can
use trigger_error("Could not perform query $query: " . mysql_error(),
E_USER_ERROR);
or something similar to that.

A simpler short-term option would be to replace die("...") with:
die("An error has occured." . error_log("Could not perform query
$query: " . mysql_error());

I'm not sure what error_log returns, but it probably won't be
particularly useful, nor dangerous to expose to the Bad Guys.

The Bad News is that you should count on spending about 3 X as much
time thinking about Security to write your application as you would
have spent if everybody in the world was Good People.

-- 
Some people have a "gift" link here.
Know what I want?
I want you to buy a CD from some starving artist.
http://cdbaby.com/browse/from/lynch
Yeah, I get a buck. So?

-- 
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to