Security advice needed

[Potatis]

I am updating old code in a website, converting to the mysqli syntax from mysql. I am learning about OOP as well, and I recently asked for help with a database class, but this query is about procedural php/mysqli code.

 

This login script works, but I am not happy with the mysqli_real_escape_string() function. For one reason, it doesn't really work. When I test for sql injection with the firefox plugin, it writes some crap in one of my tables, not even related to logging in. I also get a large number of "Server Status Code: 302 Found" messages when I check, if that is meaningful to anyone.

 

The old function I used with the mysql syntax doesn't work any better. I have read about prepared statements, but that seems to deal with INSERT, and all I am doing is querying the database to see if a username and matching password exists. Still if people are trying to hack in, they'll be trying to INSERT.

 

Here is the login script which does log a user in if the name and password is correct, and redirects them to the login page if there is a login returns false.

 

<?php
session_start();

require_once("connection.php");

if($_SERVER['REQUEST_METHOD'] == "POST") {
   
   $username = mysqli_real_escape_string($connection,$_POST['username']);
   $password = mysqli_real_escape_string(md5($connection,$_POST['password']));

   $result = mysqli_query($connection,"SELECT * FROM siteusers WHERE username='$username' AND password='$password'") or die(mysqli_error($connection));
   if(mysqli_num_rows($result) > 0) {
      $_SESSION['is_logged_in'] = 1;
   }
}
if(!isset($_SESSION['is_logged_in'])) {
   header("Location:../login.php");
} else {
   header("Location:../index.php");
}

//Assign username to $_SESSION['username'] for logging.

if(mysqli_num_rows($result) > 0) {
   $_SESSION['is_logged_in'] = 1;

   $_SESSION['username'] = $_POST['username'];

}
?>

 

Any suggestions regarding what to do from here to help make this script more secure?

[the182guy]

mysqli::real_escape_string() and mysqli_real_escape_string are secure enough to prevent SQL Injection.

 

You can use MySQLi Prepared Statements for SELECT queries, not just INSERT queries. See here:

http://devzone.zend.com/article/686#Heading10

 

This is not a security issue but a performace one.... you are selecting all fields from the table, it would be better to change it to just select the username and use that to store in the session instead of the posted value. You could also add LIMIT 1 to the query because you will only select one record.

 

As for other security, have a read on PHP Session Security, you could do something to help prevent Session Fixation and Session Hyjacking.

 

One easy way to help prevent this is use session_regenerate_id() before you set $_SESSION['is_logged_in'] to 1.

 

A rule of thumb is to regenerate the session ID each time the users security level changes, in this case there is only two security levels which are not logged in and logged in.

[Potatis]

Thanks for the info. I read that same link you sent me, earlier today and tried to get my code to work with it, and it did work. It logged in when it should and redirected when the login was false. I was uncertain of how good it was though, perhaps I should stop using this Firefox plugin, but the plugin was still able to inject data into one of my tables, and the same failures were there as with the mysql_real_escape_string, and the old clean() function I used to use. That's what has me so confused, I can find no way of stopping "sql inject me" to give my script a clean bill of health.

 

This is not a security issue but a performace one.... you are selecting all fields from the table, it would be better to change it to just select the username and use that to store in the session instead of the posted value. You could also add LIMIT 1 to the query because you will only select one record.

 

Yes, I should do this and I will. There is no need to select * and I only need limit to one record. Thanks for that.

 

Thanks for the tip, I will read about session_regenerate_id() now.

 

 

[the182guy]

I don't know what your firefox plugin is doing but (if using real_escape_string) I honestly doubt that you can give me a string that someone can enter into the username or password field, which would let the login (without correct user or pass).

[Potatis]

It doesn't allow a login, it redirects to the login page as it should. But it allows some junk to be inserted in one table that I can see with phpmyadmin. Random stuff like this: 1' AND non_existant_table = '1 where it has tried to do it's hacking.

[Ken2k7]

Change this:

$password = mysqli_real_escape_string(md5($connection,$_POST['password']));

 

to:

$password = md5(mysqli_real_escape_string($connection, $_POST['password']));

 

Does that work?

[PFMaBiSmAd]

Which FF plugin are you using? The most common one 'SQL Inject ME' does not attempt to inject an INSERT query and the mysqli_query() statement does not support multiple queries anyway. You must have some code on some page that is doing an INSERT by default whenever that page is requested.

 

For that last post above, since an md5 value does not contain any special chars that could break a query, there is no point in using mysqli_real_escape_string() with an md5() instruction.

[Potatis]

since an md5 value does not contain any special chars that could break a query, there is no point in using mysqli_real_escape_string() with an md5() instruction.

 

Thanks, I had wondered whether the password needed the mysqli_real_escape_string.

 

I was using "SQL Inject Me". I have no INSERT statements at all on any page of this site that the user sees, it is a simple site that has a login. There is nothing in my login script that inserts data to the database. Content is loaded from the database on a few pages, but it is only selecting data and echoing on the screen. Nothing goes into the database. The only page that requires user input is this login page, and I have posted the script in full here.

 

The table that is getting the inserts CAN have data inserted into in from the admin control panel. I'll check that page to see if it is automatically inserting data when it is loaded. It baffles me though how SQL inject me could find the processing page from the front login page though!  ???