1

Possible Duplicate:
Best way to prevent SQL Injection in PHP

I'm new to PHP. I've heard a lot (and read about) SQL-injection attacks. I have got accustomed to the following way of writing PHP code. Could someone tell me if this is prone to SQL attacks. Also, in what way can I improve this code against SQL-attacks?

<?php
    if($_POST['submit']){
        $email = protect($_POST['email']);
        $password = protect($_POST['password']);
        $md5password=MD5($password);
        if(!$email || !$password){
            echo '<span style="color: red;" /><center>You need to fill in your <b>User Name</b> and <b>Password</b>!</center></span>';
        }else{
            $res = mysql_query("SELECT * FROM `employer` WHERE `email` = '".$email."'");
            $num = mysql_num_rows($res);
            if($num == 0){
                echo '<span style="color: red;" /><center>The <b>E Mail ID</b> you supplied does not exist!</center></span>';
            }else{
            $res = mysql_query("SELECT * FROM `employer` WHERE `email` = '".$email."' AND `password` = '".$md5password."'");
            $num = mysql_num_rows($res);
            if($num == 0){
                echo '<span style="color: red;" /><center>The <b>Password</b> you supplied does not match the one for that E Mail ID!</center></span>';}else{
                $row = mysql_fetch_assoc($res);
                $_SESSION['uid'] = $row['id'];
                echo "<center>You have successfully logged in!</center>";
                $time = date('U')+50;
                mysql_query("UPDATE `employer` SET `online` = '".$time."' WHERE `id` = '".$_SESSION['uid']."'");
                mysql_query("UPDATE employer (date) VALUES (NOW())");
                header('Location: loggedin_employer.php');
                }
            }
        }
    }
?>
Community
  • 1
  • 1
user188995
  • 447
  • 4
  • 10
  • 17
  • 2
    You should stop using `mysql_*` functions. They're being deprecated. Instead use [PDO](http://php.net/manual/en/book.pdo.php) (supported as of PHP 5.1) or [mysqli](http://php.net/manual/en/book.mysqli.php) (supported as of PHP 4.1). If you're not sure which one to use, [read this article](http://net.tutsplus.com/tutorials/php/pdo-vs-mysqli-which-should-you-use/). – Matt Aug 16 '12 at 13:30
  • 1
    You could improve it further by moving to [PDO](http://php.net/manual/en/book.pdo.php) and using prepared statements - it would also further protect you from the `mysql_*` functions becoming obsolete. PDO is the new *shiny* in PHP database connectivity. – Fluffeh Aug 16 '12 at 13:31
  • If you want a critique of that particular code, you should probably be asking on http://codereview.stackexchange.com/ … and provide a copy of the `protect` function you are calling. – Quentin Aug 16 '12 at 13:32
  • 1
    Unrelated to the question, but MD5 is insufficient protection for passwords. See the [FAQ](http://php.net/manual/en/faq.passwords.php) – Quentin Aug 16 '12 at 13:33

1 Answers1

4

You should stop using mysql_* functions. They're being deprecated. Instead use PDO (supported as of PHP 5.1) or mysqli (supported as of PHP 4.1). If you're not sure which one to use, read this article.

Also, read this exchange: How can I prevent SQL injection in PHP?

Community
  • 1
  • 1
Matt
  • 6,993
  • 4
  • 29
  • 50