Is this mySQL code safe form attack?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
king spengo
Forum Newbie
Posts: 8
Joined: Sat Jun 16, 2007 7:13 am

Is this mySQL code safe form attack?

Post by king spengo »

I'm pretty new to php and recently discovered mysql_real_escape and would just like to ask if this code is safe from attack.

Code: Select all

function check_input($value)
{
// Stripslashes
if (get_magic_quotes_gpc()) {
$value = stripslashes($value);
}

// Quote if not a number
if (!is_numeric($value)) {
$value = "'" . mysql_real_escape_string($value) . "'";
}

return $value;
}

// include database configuration file
include("config.php"); 

// assign username, password value to variables
$username = $_POST["username"];
$password = $_POST["password"];

// encrypt the password
$encrypted = md5($password);

// connect to the mysql server 
$link = mysql_connect($server, $user, $pass)  or die ("Could not connect to server"); 

// select the database 
mysql_select_db("database", $link) or die ("Unable to select database!");

$username = check_input($username);
$encrypted = check_input($encrypted);

// match the given password against password in database
$match = mysql_query("SELECT username FROM users WHERE username = $username AND user_password = $encrypted;"); 
$rows = mysql_num_rows($match);

// if the given username does not exist
if ($rows <= 0) {
echo "You have specified an incorrect or inactive username, or password is incorrect.<br>"; 
echo "<a href=index.php>Try again</a>"; 
exit; 
}

// if the given username does exist
else {

// register the session
session_register("username");

// close the connection
mysql_close($con);

// redirect to index.php
header('Location: index.php');
}
User avatar
kaszu
Forum Regular
Posts: 749
Joined: Wed Jul 19, 2006 7:29 am

Post by kaszu »

It looks safe against SQL injections.
session_register works only if register_globals are on, so you should use $_SESSION['username'] = $username instead.
Z3RO21
Forum Contributor
Posts: 130
Joined: Thu Aug 17, 2006 8:59 am

Re: Is this mySQL code safe form attack?

Post by Z3RO21 »

king spengo wrote:

Code: Select all

// encrypt the password
$encrypted = md5($password);
You should look into salts because this is open to simple dictionary attacks.
lafflin
Forum Contributor
Posts: 123
Joined: Thu Jul 26, 2007 6:26 pm

Post by lafflin »

I'm a newb too, but I would like to weigh in on this because I'm here to learn as much as possible.

This script is an include stored outside of your public directory right?

also, I'm using a similar mysql_real_escape_string, except mine makes no exception for numbers. What is that for? I was just wondering if mysql_real_escape_string by itself, is enough security for basic form inputs before being stored into my DB.
User avatar
xpgeek
Forum Contributor
Posts: 146
Joined: Mon May 22, 2006 1:45 am
Location: Kyiv, Ukraine
Contact:

Post by xpgeek »

Hi king spengo You sql qeury look safe ;-)

Shoot several rules to make you code safety:
1)Separate you code into several small pieces for example: take sql connection to config.php and move you config.php up from public directory also move all function and classes into separate files.
2)Start using placeholders in every you sql query;
3)Read this http://shiflett.org/php-security.pdf
king spengo
Forum Newbie
Posts: 8
Joined: Sat Jun 16, 2007 7:13 am

Post by king spengo »

Thank you for your psts and suggestions.
king spengo
Forum Newbie
Posts: 8
Joined: Sat Jun 16, 2007 7:13 am

Post by king spengo »

lafflin wrote:This script is an include stored outside of your public directory right?

also, I'm using a similar mysql_real_escape_string, except mine makes no exception for numbers. What is that for? I was just wondering if mysql_real_escape_string by itself, is enough security for basic form inputs before being stored into my DB.
So far the only include is for the config file for database connection and about mysql_real_escape_string making exception for number I'm not to sure myself, found this script on a tutorial website.
lafflin
Forum Contributor
Posts: 123
Joined: Thu Jul 26, 2007 6:26 pm

Post by lafflin »

Ok, At first I saw that you had defined the check_input function but I didn't see where you actuallu used it. I see it now though. I actually learned from my research to define that function within my DB connect include that way I can refer to it in all of my scripts since it's always going to be a popular function.
What about username? should you use the function there too? I am asking because I am new and I really don't know. Is there a reason not to use it there, or on any other inputs? Or should it be used for all inputs? what about preselected drop down menu inputs, radio buttons and those sort of form inputs, should it be used there too? Maybe I should just research it more, but if anyone wants to share their knowledge thats cool too.
Post Reply