Worried about SQL Injection. Is My Code Correct?

Question

I am new to SQL queries and I have been learning it based on the info available on the web and in this site itself varying from info posted since 2005 to this year. I am creating a php application that inserts data to MySql database. When I was researching about Sql injection, there was a lot of info on the web. I read various methods to implement it such as using mysql_real_escape_string, Preparing statements and parameterized queries using sqli or PDO etc.. I have put together what I've learnt and went with the sqli method for prepare and parametrized queries. Before I release it on my website, I just want to make sure that I have done it correctly and if this should prevent SQL injection. I am really nervous about this and it will be hugely helpful if someone can let me know if this following queries are safe for me to have it on my live site. I am newbie on this and just need a re-assurance:

I have the following code now:

//Connect to Database

$user_input_Name = $_POST['Name'];
$user_input_date = $_POST['date'];
$user_input_info = $_POST['info'];
$user_input_resume = $_POST['resume'];
$user_input_experience = $_POST['experience'];
$user_input_dateGraduated = $_POST['dateGraduated'];

//Preparing the query 
$stmt = $mysqli->prepare("INSERT INTO our_weekend_template (name, date, info, resume, experience, date_graduated) VALUES (?, ?, ?, ?, ?, ?)");

// Check that $stmt creation succeeded
// prepare() can fail because of syntax errors, missing privileges, ....
if ( false===$stmt ) {
  // and since all the following operations need a valid/ready statement object
  // it doesn't make sense to go on
  // you might want to use a more sophisticated mechanism than die()
  // but's it's only an example
  die('prepare() failed: ' . htmlspecialchars($mysqli->error));
}

// "s" means the database expects a string
$rc = $stmt->bind_param("ssssss", $user_input_Name, $user_input_date, $user_input_info, $user_input_resume, $user_input_experience, $user_input_dateGraduated );

// bind_param() can fail because the number of parameter doesn't match the placeholders in the statement
// or there's a type conflict(?), or ....
if ( false===$rc ) {
  // again execute() is useless if you can't bind the parameters. Bail out somehow.
  die('bind_param() failed: ' . htmlspecialchars($stmt->error));
}

$stmt->execute();

// execute() can fail for various reasons. And may it be as stupid as someone tripping over the network cable
// 2006 "server gone away" is always an option
if ( false===$rc ) {
  die('execute() failed: ' . htmlspecialchars($stmt->error));
}

$stmt->close();


//Disconnect from database

What I am doing here is I am using the prepare, bind_param and execute to process the queries and then added an error for each step of it so if it fails so its useful for debugging.

These are the pages I referred to to put the above code together:

How can I prevent SQL injection in PHP?

MySQLi prepared statements error reporting

http://www.wikihow.com/Prevent-SQL-Injection-in-PHP

Is it okay and is it safe for me to have the code as it is on my live site? Have I missed on anything? Do I need to improve anything to increase the security?

I think Code Reviews shouldn't be on SO Go to codereview.stackexchange.com . But at a glance I can't see anything wrong with yours. — Amber, Nov 22 '13 at 13:03
Thanks for your comments. I wasnt aware of codereview.stackexchange.com. I will surely use that in the future. Cheers! — Neel, Nov 22 '13 at 13:06
possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — vascowhite, Nov 22 '13 at 13:16
I think your code is over commented, it makes it hard to read. Otherwise it looks fine. — vascowhite, Nov 22 '13 at 13:18
yeah I know..lol.. i have these comments so its easier for me to understand whats happening if I refer to it again in the future. :) — Neel, Nov 22 '13 at 13:23

score 1 · Accepted Answer · answered Nov 22 '13 at 13:03

1

Prepared statements put a hold on SQL injection, so yes, you're doing it right! Your error handling is excellent too :)

Slightly offtopic:

In PHP you don't have to explicitly compare a value to false. Everything in an if-statement automatically gets parsed to a boolean. So a non-empty object returns true, while false clearly returns... false.

if(!$stmt) die();

However, there's absolutely nothing wrong with your code though.

answered Nov 22 '13 at 13:03

Tim S.

13,597
7
46
72

Thank you so much Tim. Re-assurance from all of you have put me to ease. I am very anxious about this when I was reading the materials the past few days. With the error handling, I actually got from my another topic here where VolkerK had suggested these and I loved that method. I will surely take your input on php and it makes sense. I will edit my code with that and it simplifies it too. Thanks again Tim. Much Obliged! :) – Neel Nov 22 '13 at 13:11
2

@TimS Some mysqli functions (and PDO too) can return `0` or `NULL` which would evaluate to `false`. For these an explicit comparison against `false` is the only way to differentiate between an `error` and `nothing happened`. I'd leave the tests as they are: it's absolutely clear what's going on. – Nov 22 '13 at 13:17
@MikeW True, but `mysqli::prepare()` returns either an object or false, `mysqli_stmt::bind_param` returns either true or false and `mysqli_stmt::execute` returns true or false too. So in this case it would be perfectly fine. But yeah, you're still right and it **should** be mentioned :) – Tim S. Nov 22 '13 at 14:23

Worried about SQL Injection. Is My Code Correct?

1 Answers1