What is wrong with this code?

[lordvader]

Hi,

I've written a basic PHP script that recieves a character from an AJAX function and then searches through a database to look for words begining with that character. Any words that are found are passed back to the AJAX function.

The code all works and the page does what I want it to do, but I've been told that I need to add some slashes or do some escaping (whom I cannot ask for further assistance), and I'm not sure where this would need to be done or why.

Here's the PHP:

<?php

$host = "localhost";
$user = "root";
$password = "mypassword";
$database = "mydata";
$server = mysql_connect($host, $user, $password);
$connection = mysql_select_db($database, $server);
$param = $_GET['query'];
$query = mysql_query("SELECT * FROM products WHERE product LIKE '$param%'");

for ($x = 0; $x < mysql_num_rows($query); $x++) {
$row = mysql_fetch_assoc($query);
$output = $row['product']."\n";
echo $output;
}

mysql_close($server);
?>



I don't do anything more than dabble with PHP unfortunately, so any help at all would be hugely appreciated

[NogDog]

Probably the main concern is using the value from $param variable in your query directly from the value received in the URL query string (via the $_GET['query'] value). This means that a hacker could very easily attempt a SQL injection (http://www.php.net/manual/en/security.database.sql-injection.php) attack via that parameter.

Take a look at the mysql_real_escape_string (http://www.php.net/mysql_real_escape_string)() for a means to escape any characters within a string that have special meanings within a MySQL query.

[afigueroa]

try this:


/* connect */
$con = mysql_query("host","username","pass");
/* select database */
mysql_select_db("db",$con);
/* check connection */
if($con) {
$query = mysql_query("SELECT * FROM products WHERE product LIKE '".$param."%'");
$numRows = mysql_num_rows($query);
/* check if there was any data retrieved */
if($numRows > 0) {
/* send data through a loop for echoing */
while($data = mysql_fetch_array($query)) {
echo $row['product'];
}
}
else {
echo "No data.";
}
}
else {
echo "error connecting: ".mysql_error();
}

[lordvader]

Brilliant, thanks guys,

So if I change the middle bit of my code to this:

$server = mysql_connect($host, $user, $password);
$connection = mysql_select_db($database, $server);
$param = $_GET['query'];

/* use mysql_real_escape_string */
mysql_real_escape_string($param, $server);

$query = mysql_query("SELECT * FROM countries WHERE country LIKE '$param%'");

/* etc */


Then everything will be ok? The page still works with the new line of code so I guess it's either working correctly or not doing anything at all!?

[NogDog]

That should prevent any sort of SQL injection attack. The other issue you might want to consider is if you want to do any sort of input validation. For instance, the stated purpose is to receive a character and then search for matches that begin with that character. What if it receives more than one character, or an empty string? And is any character to be allowed, or only letters, or letters and numbers, or what?

[lordvader]

If more than 1 character is entered, a new request is initiated so the $param variable will hold both characters. The page works with two characters in the field

I was going to do validation on the client-side so that if non-allowable charcters are entered, the request doesn't even occur, but thanks for the advice

[NogDog]

Client-side validation won't happen if the user has JavaScript disabled, and a malicious user can easily bypass it. I don't see that allowing it would cause anything "bad" to happen since your are escaping any problematic text they might send in terms of SQL injection, I just want you to keep it in mind should you think of any reason that you must limit the number or types of characters submitted.

[lordvader]

good point, thanks