Did you not see this…
…during your extensive searching? 
cootheead
Thank you coothead
But it seems too complicated to change one single line of code. I’ll keep looking.
One thing no one has brought up is that you are not properly escaping your input. You have to escape the data that you receive in the request before including it in your SQL statement or else it could change the intent of your query. For example:
$password = "' OR '' = '";
$query = "select 1 from users where username = '$username' and password='$password'";
// ^ this could result in someone getting logged in for any user
This is called SQL injection. To protect against injection with a LIKE, you should escape quotes as well as the special pattern matching characters.
Here’s something you can use:
function escapeForMysqlString ($str, $escapeForPattern = false) {
$replacements = array(
"\\" => "\\\\",
"'" => "\\'",
'"' => '\\"',
"\x00" => "\\0",
"\n" => "\\n",
"\r" => "\\r",
"\x1a" => "\Z",
"\b" => "\\b"
);
$patternReplacements = array(
"_" => "\\_",
"%" => "\\%"
);
foreach($replacements as $search => $replace) {
$str = str_replace($search, $replace, $str);
}
if ($escapeForPattern) {
foreach($patternReplacements as $search => $replace) {
$str = str_replace($search, $replace, $str);
}
}
return $str;
}
In your case, you would do
$searchq = escapeForMysqlString($searchq, true);
before including $searchq in the string. Otherwise, if $searchq contained something like a single quote, the input could cause a functionally different query to run. That is a big security hole.
Hi Amadeus
Thanks
This is getting more and more complicated! I don’t know where to start, but I’ll get there in the end.
Thank you again. I’ll have a good look at this tomorrow morning. The brain is too tired now,
You don’t escape input - you validate or sanitize input.
Escaping is for output when there is no way to keep the data separate from the code and the output can contain characters that could be misinterpreted as code. Escaping is always specific to the use being made of the data and so should be done last after all the rest of the processing.
This is mostly a matter of semantics. I didn’t mean to imply that the input should be escaped once and then used in different outputs (e.g. both a SQL query and HTML). But in the post given, the variable $searchq was not used after the query, so I was saying they should do something like:
$searchq = escapeForMysqlString("...", true);
$query = "...";
If they used $searchq later as part of the HTML message, then they would have to save the unescaped version
Perhaps I confused you with my line about replacing $searchq. It was meant to occur just before the creation of the query. And only apply inside the query. The variable $searchq is not used in the example after the creation of the query, so overwriting it is not a problem.
In my own code, I do something like this:
$query = prepQuery(
"SELECT * FROM Chains
WHERE Country LIKE :%searchq
OR City LIKE :%searchq",
array("searchq" => $searchq)
);
When dealing with user inputs, you should be using prepared statements. SQL Injection doesn’t come from user inputs, it comes from bad coding.
So are you telling me that
IamJeff`nj$!~\/*(^#$_
Should be escaped simply because it has all special characters?
No it shouldn’t. If you escape all of the special characters and maybe even change it by removing all of the special characters, you make the password even more weaker than it used to be and the fact that it isn’t the original user’s password which is a bad thing to do. This makes it easy for hackers to brute force and maybe even rainbow tables.
Using prepared statements already separates what is code and what is user input. You don’t need to escape anymore when properly using prepared statements. You just then need to sanitize and validate the user input to make sure that it is your standards.
Hi spaceshiptriooper
Nice to see you back!
Did you have a chance to look at my file?
Thanks
This is the complete file. I made some changes to mysqli but don’t trust them…
<?php
$user_name = "pintotou_****";
$password = "****";
$database = "pintotou_search";
$server = "localhost";
mysql_connect("localhost","pintotou_****","*****") or die ("could not connect");
mysql_select_db("pintotou_search") or die ("could not find db");
$output = '';
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
$searchq = $_POST['keyword'];
/*$searchq = preg_replace("#[^0-9a-z]#i","",$searchq);*/
$query = mysql_query("SELECT * FROM Chains WHERE Country LIKE '%$searchq%' OR City LIKE '%$searchq%'") or die ("could not search");
$count = mysql_num_rows($query);
if($count == FALSE)
{
$output = 'There was no search results!';
}
else
{
while($row = mysql_fetch_array($query)) {
echo <<<EOD
<table>
<tr>
<td>Chain : {$row['Chain']}</td>
<td>Country : {$row['Country']}</td>
<td>City : {$row['City']}</td>
<td>Top : {$row['Top']}</td>
<td>Medium : {$row['Medium']}</td>
<td>Low : {$row['Low']}</td>
</tr>
</table>
EOD;
// The above must be on a line by itself with no leading space of any kind
}
}
echo "<p><a href='End.php?page=post&&post=$post_Chain'>$post_City</a></p></ br>";
}
?>
I sent you an earlier version. This is the one where I changed some of the mysql code
<?php
$user_name = "pintotou_****";
$password = "***";
$database = "pintotou_search";
$server = "localhost";
mysqli_connect("localhost","pintotou_****","****","pintotou_search" ) or die ("could not connect");
$output = '';
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
$searchq = $_POST['keyword'];
/*$searchq = preg_replace("#[^0-9a-z]#i","",$searchq);*/
$query = mysqli_select_db("SELECT * FROM Chains WHERE Country LIKE '%$searchq%' OR City LIKE '%$searchq%'") or die ("could not search");
$count = mysqli_result::$num_rows($query);
if($count == FALSE)
{
$output = 'There was no search results!';
}
else
{
while($row = mysqli_fetch_array($query)) {
echo <<<EOD
<table>
<tr>
<td>Chain : {$row['Chain']}</td>
<td>Country : {$row['Country']}</td>
<td>City : {$row['City']}</td>
<td>Top : {$row['Top']}</td>
<td>Medium : {$row['Medium']}</td>
<td>Low : {$row['Low']}</td>
</tr>
</table>
EOD;
// The above must be on a line by itself with no leading space of any kind
}
}
echo "<p><a href='End.php?page=post&&post=$post_Chain'>$post_City</a></p></ br>";
}
?>
I’ve already looked at it. There are still a lot of problems with your files. After you have checked if the form was submitted, you should then make sure that the user gets redirected since the form was not submitted after if($_SERVER['REQUEST_METHOD'] == 'POST').
Also, I still see the * (asterisk) wild card in your query. You should only need to specify which columns you are looking for or are going to display.
Here is a more cleaner version of your snippet. I’ve added proper spacing with the right syntax. I’ve also added comments for you to look at. I’m not sure if it’s going to be 100% right, but it’s a start.
<?php
// Your database preferences
// We are using constants instead of variables for this
// You can use either or
define('HOST', 'localhost'); // Database host
define('USERNAME', 'root'); // Database username
define('PASSWORD', 'root'); // Database password
define('DATABASE', ''); // Database
$mysqli = new mysqli(HOST, USERNAME, PASSWORD, DATABASE); // Connect to the database using MySQLi_* OOP and constants
if($mysqli->connect_errno) {
// Do not die or display any MySQL errors in this area.
die('Unable to connect to the mysql server');
}
// Check to see if the form was submitted
if($_SERVER['REQUEST_METHOD'] == 'POST') {
$searchq = filter_var("%{$_POST['keyword']}%", FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_HIGH); // Sanitize the string
$sql = "SELECT Chain, Country, City, Top, Medium, Low FROM Chains WHERE Country LIKE ? OR City LIKE ?"; // Your query string
$prepare = $mysqli->prepare($sql); // Prepare your query string
$prepare->bind_param('ss', $searchq, $searchq); // Bind the placeholders to your search variables
// s = string | i = integer | d = double | b = blob
$prepare->execute(); // Execute the prepared statement
$prepare->store_result(); // Store the results for later checking
// Use num_rows to check if the results return a 0 or 1. 0 meaning false and 1 meaning true
if($prepare->num_rows) {
$prepare->bind_result($chain, $country, $city, $top, $medium, $low); // Append variables to the columns you specified
// Throw them all in a while loop
while($prepare->fetch()) {
// Technically you don't need to use HEREDOCS because if you end the PHP tag and restart it
// You can have your full HTML in your PHP file
// This isn't ideal though, you should separate your HTML from PHP, but since you are still a beginner
// This doesn't really mean anything.
// I'd say put these results in a new PHP file and require it
?>
<table>
<tr>
<td>Chain : <?php echo $chain; ?></td>
<td>Country : <?php echo $country; ?></td>
<td>City : <?php echo $city; ?></td>
<td>Top : <?php echo $top; ?></td>
<td>Medium : <?php echo $medium; ?></td>
<td>Low : <?php echo $low; ?></td>
</tr>
</table>
<?php
}
} else {
echo "There was no search results!"; // Tell the user there are no results
}
// echo "<p><a href='End.php?page=post&&post=$post_Chain'>$post_City</a></p></ br>";
// No idea why this is needed, but I commented this out.
} else {
header('Location: http://pintotours.net/TEMP/TEST/test1.html'); // redirect the user if the form was not submitted.
}
What are you talking about? Escaping is simply so that it doesn’t change the SQL statement. I’m not talking about changing " to actually be \" in the database. Escaping it in the SQL statement simply allows you insert " into the database. Prepared statements are great, but THEY STILL DON’T ESCAPE special pattern characters. If a user puts a “_” or a “%” in their input, and you just feed it to the statement with “%” around it, you are going to change the functionality of your statement.
Prepared statements don’t escape special pattern characters.
To re-emphasize:
$input = '%bar';
$statement = $mysqli->prepare('select foo where baz like ?');
$statement->bind_param('s', $input . '%'); // oops
In this example, since we didn’t escape the special pattern character ‘%’, it changes the meaning of our query to do a match on both the front and end of the user’s data, which will be a lot slower of a query. And it means that if the user was really searching for something that had a literal ‘%’ in it, they’re not going to get the right answer.
Now, doing both prepared statements and escaping for patterns is fine. Do that. But OP was originally using the mysql_ extension, and here, the escaping has to be done anyway, so I didn’t see any reason to complicate things more with both prepared statements and a pattern escape.
I don’t have a mis-understanding of it. I know what it does and what it’s used for. But did you read this line yet?
Using prepared statements already separates what is code and what is user input.
By using prepared statements, you don’t need to escape the string because the SQL part is going to be separate from the user input. I could be wrong, but the only person who can actually change the PHP source code is OP since no one has permissions to edit the file. So in your example, the $input . '%' part is nulled because the user input is going to be sanitized before the SQL string gets executed. Meaning that basically the user input will become %bar% instead of %bar. Also, I’d like to link this commented made via the PHP manual documentations for mysqli_real_escape_string.
http://php.net/manual/en/mysqli.real-escape-string.php#102639
Obviously OP wants to convert to MySQLi_* so that means that OP should be learning when to use prepared statements and when not to.
Also, I would like to ask. Why are you re-inventing the wheel with your escapeForMysqlString function? Shouldn’t you be using mysqli_real_escape_string?
Aside from that, I’ll repeat it a million times. If you use prepared statements, you don’t need to escape the string because PHP will separate the user input from the SQL string. This means that you shouldn’t be feeding the arbitrary string into the actual SQL query string. That is what allows SQL Injection to occur in the first place. It’s because you aren’t using prepared statements and you are explicitly stuffing the actual user input into the SQL string.
So going back on your first example
$query = "select 1 from users where username = '$username' and password='$password'";
This will lead to SQL Injection because you are stuffing the actual user input into the string. If you use prepared statements you aren’t deliberately stuffing the actual user input into the string. Like so.
$query = "select 1 from users where username = ? and password = ?";
And you don’t have to escape the user input because prepared statements will actually separate the string from the user input.
An example with SQL Injection using your first example.
$username = "1' or '1' = '1";
$password = "1' or '1' = '1";
$query = "select 1 from users where username = '$username' and password='$password'";
This will result in exploiting your code. But with prepared statements. This is what happens.
$username = "1' or '1' = '1";
$password = "1' or '1' = '1";
$query = "select 1 from users where username = ? and password = ?";
$prepare = $mysqli->prepare($query);
$prepare->bind_param('ss', $username, $password);
$prepare->execute();
$prepare->store_result();
if($prepare->num_rows) {
print('A result was found');
} else {
print('No results were found');
}
The result with using prepared statements is: No results were found. So the user input is actually just a string and not part of the query string.
Correct me if I’m wrong.
Escaping is not needed for that. Simply use a PREPARE statement so as to keep the SQL and the data completely separate - then it doesn’t matter what is in the data as long as it is valid.(and even invalid data will not cause an issue with the database call - it will just result in junk data.
1 Like
Hi spaceshiptrooper
You saved my life!
Yes, it connects, and displays the data on-screen. And it looks very professiopnal!
http://pintotours.net/TEMP/DB/test10.html (try to enter Brussels, or Paris, or Lisbon)
I am not sure if I understood the end of the code you sent me regarding html. On this, I failed to tell you that the Search box page and the Results page are two different ones. I made a quick, rough sketch of the Results page which is here http://pintotours.net/TEMP/TEST/End.php. When the results arrive it should look like this: one row if there is only one result, like in Paris; 3 rows if there 3 results like in Brussels, and so on.
Am I to understand that I should clear all the content of #main in http://pintotours.net/TEMP/TEST/End.php and place the end of your code there instead?
Chain : <?php echo $chain; ?> Country : <?php echo $country; ?> City : <?php echo $city; ?> Top : <?php echo $top; ?> Medium : <?php echo $medium; ?> Low : <?php echo $low; ?> <?php }<table>
} else {
echo “There was no search results!”; // Tell the user there are no results
}
// echo "<p><a href='End.php?page=post&&post=$post_Chain'>$post_City</a></p></ br>"; // No idea why this is needed, but I commented this out.
} else {
header('Location: http://pintotours.net/TEMP/TEST/test1.html'); // redirect the user if the form was not submitted.
}
This line
header('Location: http://pintotours.net/TEMP/TEST/test1.html'); // redirect the user if the form was not submitted.
seems to indicate that the results go back to the Search page, but as I mentioned, this is not the case. So, should I change it to the End.php ?
Many, many thanks
PS - For some reason the table <tr> and <td>s don´t show above in the quote…
Discourse strips out some tags and the code won’t display
What I’ve found works best for me is to put 3 backticks on their own line before the code block and 3 backticks on their own line after the code block
My guess is you’re developing the code without this at the beginning
<?php
error_reporting(E_ALL);
ini_set('display_errors', true);
else you would be seeing a “headers already sent” error message
Hi Mittineague
All this is a big mystery (and adventure) for me!
Where should that code be? In the End.php, or the test10.html ?
I put it at the very top of the php script but all it does is still dispaly the results on a white page. Nothing else
At the beginning of the PHP file with header() in it.
Just remember to remove it or comment it out after you’re done working up the code in the file.
Hi
You missed my editing of the last post
I put it there but I get no further information when the script returns the data. But that’s probably because the redirecting to the Results page has not been done yet, because I don’t know how…
If you go into http://pintotours.net/TEMP/DB/test10.html and enter “Brussels” you can see the result
Ahhhh… I missed that bit about header(). Where does the header() go?
This is now the top of the php file:
<?php
error_reporting(E_ALL);
ini_set('display_errors', true);
It has to be before anything (including whitespace) is output to the browser.
IMHO it is best to write the code so that it comes before any HTML is output - if possible.
I have seen others use buffer, but I have almost always been able to assign the HTML to a variable (most often using heredoc syntax) instead, delaying its output until after the header() call
Hi
I’m getting confused…
I did a new html page in what I thought was spaceshiptrooper intention using the end of his code into a new page
http://pintotours.net/TEMP/DB/Result.php
But, first I am getting php error on that page and still don’t understand how the Results of the Search are going to end up there., if the php script does not know the location
Sorry about this…