I wan to secure my PHP script

[badgoat]

Hello!

I have a simple search field on one of my scripts, and I want to make it secure, so that no one can take advantage of it. I'm relatively green to PHP and REALLY green when it comes to securing PHP, so I am not sure even what to Google to find a tutorial for securing my code. Can anyone point me in the right direction?

Thanks in advance!

[liquorvicar]

It depends what you're doing with the value the user enters in the search field, and what you're allowing the user to enter into the field.
Generally, it's always better to positively validate any input data rather than the other way round, i.e. rather than trying to eliminate any invalid values, check if it's a valid value and discard if not (there is a subtle but important difference).
I'm assuming you'll want to use the field value to search against a database. In which case, if you're letting the user enter a search string (i.e. "look for this") rather than just a word, you probably want to use one PHP's escape_string functions (e.g. mysql_real_escape_string().

[badgoat]

Hi, thank you for the reply.

The search field searches data in my db for a number, and if the number is not found, it inserts the unfound number into a different db. I will look into this PHP function. Does it still seem to be the correct one for this usage?

[liquorvicar]

It would certainly offer you some protection against SQL injection but if you're expecting a number, why not use intval() and/or is_numeric() to make sure you only pass a numeric value to your query.

[phporcaffeine]

Well by 'searching' I assume that you are only using a SELECT query, which of all the types of queries, is the safest.

mysql_real_escape_string() is really only going to help with 'data cleansing' in that, it wouldn't allow any characters in the string that would cause mysql to 'burp', such as the apostrophe.

All aside, mysql_real_escape_string isn't a bad idea. If cleansing is what your after, I would consider the following instead:

trim(stripslashes(strip_tags()));

again, in the case of a select query, usually, the worst that can happen is that MySQL doesn't return results, which is counter productive for the user thereby making an intentional malicious act less likely.

In the case of a select query, I would be more worried that a user could query in a way that would cause MySQL to return so many results as in to cause MySQL to lock up.

In your situation, I would look more closely at how to 'tighten up' my query statement (Ex. use the LIMIT clause in the query statement) and not so much the form itself.

-phpORcaffeine

[badgoat]

I am using an if / else statement where if the script does not find a match it does:

$anymatches=mysql_num_rows($data);
if ($anymatches == 0)
{
$date=date('D M j Y G:i:s');
$sqlquery = "INSERT INTO todo VALUES('','$find', '$date')";
$queryresult = mysql_query($sqlquery) or die(" Could not execute mysql query !");

so my worry is that since an injection attack would not be found in the db since it's all numbers, the attacker would be able to execute something malicious. I believe the right step would be to harden the above, since the ELSE would be a simple select query... Correct?

[phporcaffeine]

//I Assume this returns rows that match a previous query
$anymatches=mysql_num_rows($data);

if ($anymatches == 0) {

$date=date('D M j Y G:i:s');

//IF A USER SOMEHOW GOT SOMETHING 'INVALID' INTO $find, IT WOULD EITHER INSERT
//A VALUE THAT SHOULDN'T BE OR IT WILL FAIL. IT WON'T CAUSE MYSQL TO 'DELETE
// THE DATABASE' OR SOMETHING ELSE HORRIBLE.

//A LITTE DATA CLEANSING WOULD BE OKAY THOUGH

$sqlquery = "INSERT INTO todo VALUES('" . trim(mysql_real_escape_string($find)) . "', '" . trim(mysql_real_escape_string($date)) . "')";

$queryresult = mysql_query($sqlquery) or die(" Could not execute mysql query !");

}

[badgoat]

Many thanks for the assistance 🙂

[Roger_Ramjet]

Sorry, but there is a lot of crap advise in the thread.

READ the manual at the link that Liquorvicar gave mysql real escape string and use the 'best practice' function there; it gives the full code that includes checking for is_numeric etc. Using anything else is just re-inventing the wheel and some of the methods given so far are for square wheels.

An sql injection attack can work with user input data in ANY type of query so you do not just need to protect when doing insert/update/delete but even when doing select.

Also, google for php security vulnerabilities and you will find some very good stuff out there (sorry I'm not on my usual machine or I'd give you a link now)

[liquorvicar]

The 'best practice' function would certainly help protect against SQL injection. As a matter of programming quality, I tend to break my validation down further based on what values I'm asking the user for. This helps provide more meaningful errors (e.g. they've entered text into a numeric field, or entered a space in a username field which doesn't allow spaces, or an invalid email address etc). I also think it's good practice to think about what values you're expecting on each page...

[Roger_Ramjet]

Quite agree liquorvicar, explicit in-page validation should always be done, if only to make your site more user friendly. I still recomend using the best practice function even after that though: a) to save coding stripslashes etc over and over b) because sql injection is a real threat.

I've got my bookmarks with me today so have a read of these

top 7 security blunders
session fixation
sql injection by example
form post highjacking
testing your forms for highjacking vulnerabilities

mail form exploits are the latest spammer attack and a real issue you need to address if you don't want your domain to end up on everyone's blacklists

[badgoat]

Thank you for the further advice and the links, will read up ASAP 🙂

[badgoat]

OK. So before I roll out a more secure script on a wide scale, I want to understand it smaller. So I created a simple three variable form, secured it the way I THINK it is supposed to be.... Can a guru take a look, see if I made any errors?

<?PHP
$connect= mysql_connect("#####","#####","#####")
   or die("Could not connect to database in localhost !");
mysql_select_db("ip")
   or die("Could not select that database !");

echo'
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<html>
<head>
<title>IP Converted</title>
</head>
<body>';

// $string is just a placeholder
function escapeSingleQuotes($string){
// escapse single quotes
$singQuotePattern = "'";
$singQuoteReplace = "''";
return(stripslashes(eregi_replace($singQuotePattern, $singQuoteReplace, $string)));
}

$tarp=mysql_escape_string($_POST['tarp']);
$fname=mysql_escape_string(stripslashes($_POST['fname']));
$lname=mysql_escape_string(stripslashes($_POST['lname']));

$x = split("\.",$tarp);
$conv_ip = (256*256*256*$x[0]) + (256*256*$x[1]) + (256*$x[2]) + ($x[3]);

$sqlquery = "INSERT INTO ip VALUES('','". escapeSingleQuotes($_POST['fname']) ."', '". escapeSingleQuotes($_POST['lname']) ."', '$conv_ip')";
$queryresult = mysql_query($sqlquery) or die(" Could not execute mysql query !");
$sqlquery = "SELECT * from ip";

 echo'
<table>
<tr>
    <th colspan="2">Record Added</th>
</tr>

<tr>
    <td class="selcol1">First Name</td>
    <td class="selcol2">' .$fname. '</td>
</tr>

<tr>
    <td class="selcol1">Last Name</td>
    <td class="selcol2">' .$lname. '</td>
</tr>

<tr>
    <td class="selcol1">IP</td>
    <td class="selcol2">' .$conv_ip. '</td>
</tr>


<tr>
    <td class="spacer" colspan="2"></td>
</tr>

</TABLE>
</BODY>
</HTML>
';
?>

[liquorvicar]

// $string is just a placeholder
function escapeSingleQuotes($string){
// escapse single quotes
$singQuotePattern = "'";
$singQuoteReplace = "''";
return(stripslashes(eregi_replace($singQuotePattern, $singQuoteReplace, $string)));
}

This doesn't look right to me. Why not just use mysql_real_escape_string .

$tarp=mysql_escape_string($_POST['tarp']);
$fname=(mysql_escape_stringstripslashes($_POST['fname']));
$lname=(mysql_escape_stringstripslashes($_POST['lname']));

Apart from the obvious error of missing brackets, see my comment above.
You probably want to check the value of magic_quotes before using stripslashes().

$x = split("\.",$tarp);
$conv_ip = (256*256*256*$x[0]) + (256*256*$x[1]) + (256*$x[2]) + ($x[3]);

There is a native PHP function to do this: ip2long()

$sqlquery = "INSERT INTO ip VALUES('','". escapeSingleQuotes($_POST['fname']) ."', '". escapeSingleQuotes($_POST['lname']) ."', '$conv_ip')";

Generally, it's a good idea to list your column names as part of the INSERT syntax. That way if you ever decide to add more columns, it won't break this script.

[badgoat]

Thanks for taking a look! (and I edited twice to fix a couple errors I introduced)

"This doesn't look right to me. Why not just use mysql_real_escape_string ."

the reason for using this was to allow someone with an apostrophe in the name, like Patrick O'Grady, to get into the db. With my version of PHP, mysql_real_escae_string doesn't work, so I am using mysql_escape_string instead. Does this function take care of apostrophes as well?

[liquorvicar]

badgoat wrote:

the reason for using this was to allow someone with an apostrophe in the name, like Patrick O'Grady, to get into the db. With my version of PHP, mysql_real_escae_string doesn't work, so I am using mysql_escape_string instead. Does this function take care of apostrophes as well?

mysql_real_escape_string will cope fine with names with an apostrophe and it's present in the same versions of PHP as mysql_escape_string so you must be able to get it to work.
What do you mean when you say it doesn't work?

[badgoat]

I get an error with I have the 'real_' part in there, so I Googled the error, and it said to use mysql_escape_string instead... So when I switched to that, it worked.

[liquorvicar]

mysql_real_escape_string needs a valid DB connection resource. That may be your problem. If you read the manual, mysql_escape_string is deprecated so you'd be wise to try and get it working now rather than have to go through and change it all when you upgrade to a newer version of PHP that doesn't have it.

What error do you get with mysql_real_escape_string?

[badgoat]

The error I get:

Fatal error: Call to undefined function: mysql_real_escape_string()

Per the manual, I do have a link_identifier, as I call the db connection on the same page. Not sure why I am getting this.

[liquorvicar]

How odd.
Are you using a very old version of MySQL?
Are you using the mysqli extension rather than mysql in PHP?