How to make this PHP/SQL code safer?

Question

I'm wondering how I can make this code safer, I'm using it as login.php:

if(isset($_POST["username"]) and isset($_POST["password"])) {
if($_POST["username"] == $adminusr && $_POST["password"] == $adminpass){

I want to make it a little more safe to prevent sql-injections and such. Do you have any ideas ? I'd like to learn more about making safer sites.

Thank you dudes and dudettes

Edit:

Sorry I didn't post all the code needed, but here is the complete code:

<?php
session_start();
include_once("../include/config.php");

if(isset($_POST["username"]) and isset($_POST["password"])) {
if($_POST["username"] == $adminusr && $_POST["password"] == $adminpass){

    $_SESSION['admin_user'] = $adminusr;
    $_SESSION['admin_password'] = $adminpass;
}
else {
    echo "Wrong Username or Password";
}

}else if(isset($_GET['act']) && $_GET['act']=='out') {
unset($_SESSION['admin_user']);
unset($_SESSION['admin_password']);
session_destroy (); 

}

if($_SESSION['admin_user'] == $adminusr && $_SESSION['admin_password'] == $adminpass){
$_SESSION['testdd'] = 'test';
header("location:index.php");
exit;
}

?>

Thank you once again :)

There's still nothing SQL related in there. Where does $adminusr and $adminpass come from? — Joe Phillips, May 12 '11 at 22:14
it comes from config.php where it says: $adminusr = "xxxx"; and $adminpass = "xxxx"; — MstrQKN, May 12 '11 at 22:18

score 2 · Accepted Answer · edited May 23 '17 at 12:12

If you are using $_POST['username'] in a query the following code will keep you save from SQL-injection.

$username = mysql_real_escape_string($_POST["username"]);
$password = mysql_real_escape_string($_POST["password"]);

$query = "SELECT * FROM users 
          WHERE users.username = '$username' 
          AND users.Passhash = SHA2(CONCAT(users.id,'$password'),512) ";
$result = mysql_query($query);
....

Note that you need to use mysql_real_escape_string() with mysql_query
and mysqli_real_escape_string() with mysqli_query.

Even better is to use PDO with php5, see: http://www.php.net/manual/en/book.pdo.php

Handling passwords in MySQL
Note that is strongly recommended not to store passwords in the clear in your database.
Always store a hash (preferably using SHA2) in your database and use a salt.
See: https://stackoverflow.com/search?q=hash+salt+mysql
For more info on this.

Kamil Sindi · Answer 2 · 2011-05-12T22:29:38.577

1

You should use mysqli_real_escape_string to escape special characters (assuming you are querying database). E.g. $password = mysqli_real_escape_string($link, $_POST['password']);. Otherwise, it seems fine.

edited May 12 '11 at 22:29

answered May 12 '11 at 21:49

Kamil Sindi

21,782
19
96
120

Zsipgun · Answer 3 · 2011-05-13T10:05:35.280

1

Your code doesn't show what you are actually doing with the database.

If you use a regular mysql_query, you need to encode unwanted characters, quotes (",') for example.

You also need to keep in mind, that people might want these characters in their passwords, so you need to store them in the DB encoded in some way.

Probably an MD5 hash, or just regular entity encoding (see below).

htmlentities($pass,ENT_QUOTES,'UTF-8')


(!get_magic_quotes_gpc()) ? mysql_real_escape_string($item):$item;

You should still use variable binding, that helps avoiding any SQL injections.

Check out this introduction to PDO, its really useful.

edited May 13 '11 at 10:05

answered May 12 '11 at 21:55

Zsipgun

29
6

I appreciate that you're trying to help but some of this is borderline bad advice the way you say it. For instance, filtering out " and ' is not an acceptable solution but escaping it certainly is. So you have it partially correct – Joe Phillips May 12 '11 at 22:04
Sorry if it was not understandable, by filtering out, I meant not putting it straight into a query... – Zsipgun May 13 '11 at 10:05

score 1 · Answer 4 · edited May 23 '17 at 11:55

I think you code should be like this:

if(isset($_POST["username"]) and isset($_POST["password"])) {
$handle = mysql_connect($host, $user, $pass);
mtsql_select_db($db, $handle);
$query = "SELECT user FROM table_users WHERE user='{$_POST["username"]}' AND password=MD5('{$_POST["password"]}')";
$result = mysql_query($query, $handle);
if(mysql_num_rows($result) == 1)
 {

What you need here is to escape special charcarter using mysql_real_escape_string() or PDO prepared statement, so we change this:

$input["user"] = mysql_real_escape_string($_POST["username"]);
$input["password"] = mysql_real_escape_string($_POST["password"]);
$query = "SELECT user FROM table_users WHERE user='{$input["user"]}' AND password=MD5('{$input["password"]}')";
.
.
.

You may also use addslashes() function, but it may be sufficient enough, I am quoting from this:

addslashes adds slashes to characters that are commonly disturbing. mysql_real_escape_string escapes whatever MySQL needs to be escaped. This may be more or less characters than what addslashes takes care of.

How to make this PHP/SQL code safer?

4 Answers4