-5

I'm a total PHP noob, and while I read up on MySQL injections and protecting my script, I thought I would post what I have so far here.

Below is my current (working, yet bad practice) code. Can someone offer up my weak spots, and what I can do to fix?

<?php
require_once 'login.php'; //database information

$db_server = mysql_connect($db_hostname, $db_username, $db_password)
    or die("Unable to connect to MySQL: " . mysql_error());

mysql_select_db($db_database)
  or die("Unable to select database: " . mysql_error());

$email = $_POST['email'];

$sql="INSERT INTO users (email)
VALUES ('$email')";

$result = mysql_query($sql);

if($result){
header('Location: ../thankyou.php');
}
else {
echo "ERROR";
}

mysql_close();
?> 
flash1821
  • 71
  • 6
  • 2
    Warning This extension ( mysql_query ) is deprecated as of PHP 5.5.0, and will be removed in the future. Instead, the MySQLi or PDO_MySQL extension should be used. See also MySQL: choosing an API guide and related FAQ for more information. Alternatives to this function include: mysqli_query() PDO::query() http://php.net/manual/en/function.mysql-query.php – Patashu Mar 05 '13 at 00:55

1 Answers1

0

As you're in your own words a noob there's no time like the present to pick up some good habits :)

$email = $_POST['email'];
$sql="INSERT INTO users (email) VALUES ('$email')";

is a textbook example of an SQL injection vulnerability, and this code will bite you sooner or later. The reason is that you trust the variable you receive, and hand it over to the SQL engine. Just imagine what happens if someone enters

'); drop table users; --

as their email address...

Instead of devising all kinds of sophisticated schemes to clean up the input, there's one ironclad technique that makes your code immune against injection attacks: prepared statements. As you should not start developing the deprecated mysql_ calls offered by php, you should switch to PDO or mysqli_. I would suggest PDO, which supports prepared statements. More about prepared statements here. Using PDO and prepared statments, your sample will look like this:

$stmt = $dbh->prepare("INSERT INTO users (email) VALUES (:email)");
$stmt->bindParam(':email', $email);
$stmt->execute();

You will have future proof code that's ready to be used with other database systems than MySQL and that's immune to injection attacks.

fvu
  • 32,488
  • 6
  • 61
  • 79