-1

Possible Duplicate:
Best way to stop SQL Injection in PHP

I just got this PHP script to post my form fields to a SQL table, and it's working fine. Since I plan on using this site with many users, I do not want to allow SQL-Injection's to ruin everything. I am a huge noob to PHP and I was hoping someone here could help me SQL-Inject proof my code.

       <?php

//Check for mysql connection
if (!mysql_connect("mysql4.000webhost.com","a3516066_form","lasvegas1"))
  die('Could not connect: ' . mysql_error());

//Escape SQL characters to protect against SQL injection
$username = mysql_real_escape_string($_POST['username']);
$hostname = mysql_real_escape_string($_POST['hostname']);
$ip = mysql_real_escape_string($_POST['ip']);
$email = mysql_real_escape_string($_POST['email']);

mysql_select_db("a3516066_form");

//Query to check for username match
$userCheck = "SELECT * FROM `Accounts` WHERE username = '$username'";

if(mysql_num_rows(mysql_query($userCheck)) != 0)
    die("Sorry, username is already in use. Please go back and try again.");

//If the username isn't found, insert the values
$sql = "INSERT INTO Accounts VALUES ('$username','$hostname','$ip','$email')";

if (mysql_query($sql)) {
    //Successful query
    header ("location: /done.html");
    exit();
} else 
    //Failed query
    die("Something is wrong, we could not complete your request.");
?>
Community
  • 1
  • 1
xravenz
  • 11
  • 1
  • 2
  • Your later version is safe, however, do not consider mysql_real_escape_string a magic wand that makes your data "safe". See an explanation: http://stackoverflow.com/a/9296858/285587 – Your Common Sense Feb 18 '12 at 07:23

1 Answers1

0

Never use a $_POST variable directly inside a SQL Query.

Instead, you can write a function like this, and use after connecting to a database to clean any malicious input.

function secure() {
    $_GET = array_map('trim', $_GET);
    $_POST = array_map('trim', $_POST);
    $_COOKIE = array_map('trim', $_COOKIE);
    $_REQUEST = array_map('trim', $_REQUEST);

    if(get_magic_quotes_gpc()) {
        $_GET = array_map('stripslashes', $_GET);
        $_POST = array_map('stripslashes', $_POST);
        $_COOKIE = array_map('stripslashes', $_COOKIE);
        $_REQUEST = array_map('stripslashes', $_REQUEST);
    }

    $_GET = array_map('mysql_real_escape_string', $_GET);
    $_POST = array_map('mysql_real_escape_string', $_POST);
    $_COOKIE = array_map('mysql_real_escape_string', $_COOKIE);
    $_REQUEST = array_map('mysql_real_escape_string', $_REQUEST);

}
Lucas Freitas
  • 194
  • 12
  • 1
    Seems like an awful lot of work to avoid using PDO. – sarnold Feb 18 '12 at 03:26
  • 3
    Plus you're totally trashing the superglobals on the assumption that ALL of those values are going to be used in a database context. Those superglobals should be treated as read-only at all times. – Marc B Feb 18 '12 at 03:33
  • 1
    This guy is very newbie to PHP, as you see the code sample that he sent. You know that in programming there's not wrong or correct, just bad practice and good practice. Let's the guy evolve for itself. I used this function with no problem in a couple of projects, before I turned into a more experienced programmer and using OOP and PDO correctly. Actually I'm having this issue: http://stackoverflow.com/questions/9337989/singleton-pdo-model-hierarchy – Lucas Freitas Feb 18 '12 at 03:43
  • Except "bad practice" might translate to "millions of credit cards stolen" -- granted, it's unlikely that xravenz is going to be in _that_ position soon, but you're surely familiar enough with PDO to know that usage at _this_ level is _easier_ than hand-constructed queries and completely safe at the same time. One might as well learn good habits from the start. – sarnold Feb 18 '12 at 03:54
  • To devise your own version of **magic quotes** cannot be considered a good idea in the 21th century. – Your Common Sense Feb 18 '12 at 07:19