How to make PHP input methods(get and post) safe by a simple code?

Question

This is my basic code of a login.php. It contains a basic $_POST method to get the values from form. Someone said me it is insecure. It is prone to SQl Injection. I dont want to make it complex. Please tell me how to make it more secure by simple methods.

<?php

 session_start();
 require('connection.php'); 




    $email=$_POST['username'];
     $password=$_POST['password'];


     //variables in query will be in single quotes.

     $query="select * from user_info where email='$email' and password = '$password' ";

     //fetching result

     if($result=$con->query($query))
     {


         $row =$result->fetch_array(MYSQLI_NUM); 


         if($row>0)
         {


             //creating session variables


       $_SESSION['loggedIn']=true;

           $_SESSION['email']=$row[1];
            $_SESSION['id']=$row[0];
         header("Location:profile.php");
         }

         else 
         {
             echo "flag1";

            header("Location:index.php?loginerror=invalid login");
         }



     } 
     else {
        echo "Failed";
     }

?>

Start by learning to use prepared statements with bind variables — Mark Baker, Jul 28 '16 at 19:02
**WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.2/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and never store passwords as plain-text. — tadman, Jul 28 '16 at 19:11
Although it's always an ideal to avoid making something too complex, the flip-side of this is something too simple is often completely inadequate, as is the case of the code here. — tadman, Jul 28 '16 at 19:12

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

Dangers for the database: SQL-Injections

Those are the dangers when using input data from the user to operate with the database. Some input might cause the sql-syntax to break, the database will get a badly formatted statement and run onto an error (if it was unintentionally) or even execute statements it was not supposed to (sql injection).

A possibility for unintended syntax breaking would be to ask the user for his name and he inserts something like O'Connor.

Way to treat them: How can I prevent SQL injection in PHP? tldr: prepared statements are king.

(+) I would guess most ORMs or abstraction layers between your application and your database would solve this problem on their own, should inform yourself about it tho - better safe than sorry.

Dangers for the frontend: XSS attacks

A user uses your form to input some JavaScript. When you are displaying it on your website it will get parsed and executed.

Way to treat them: use htmlentities() when displaying userdata.

(+) There are libraries like htmlpurifier which allow you to only sanitize specific elements. So if you want to let your users use HTML for a blogpost or whatever, you can a library like this which will allow certain elements and defuse the dangerous stuff.

Dangers for the filesystem: shell-injections (?)

When you are for some reason using user input to do something on your filesystem (creating a directory with the same name as the username or whatever..) someone could do the same as in sql - break your syntax and inject some commands.

Way to treat them: escapeshellcmd + Don't use user input for something like directory or filenames on your server! Put the names of the user into the database, use the id or a hash or something you generated as the filename.

Cross-Site-Request-Forgery:

Doesn't directly involve the data given to you, but I am adding it because it is something you should know about: Understanding CSRF

Remember: Treat the data based on the context in which you are using it!

There are different places for attacks and each one uses other weaknesses. So there is not the one solution to fix all the problems.

You just have to consider the context in which you are handling the data - are you saving it or displaying it.

When displaying it you want to take care of the JavaScript that might be hidden in the data so you want to escape the charakters which are special in html.

When saving data you worry only about the charakters which might break the syntax of your sql. Your database knows best which charakters to escape and how to treat which data, so just use the escaping and quoting functions provided by your database-api OR BETTER: Prepared Statements (just use them always when interacting with the database + userdata) as they work differently then normal queries and you don't have to think about escaping and quoting.

Worth noting that most frameworks have ways of mitigating all of this out of the box, so doing it the hard way and shunning those also comes with considerable risk. I'd also push the idea of prepared statements before all else, it's the most reliable, safest, cleanest way to add data to your query. — tadman, Jul 28 '16 at 20:53

score -1 · Answer 2 · answered Jul 28 '16 at 19:42

I would use PDO's. A very basic example;

<?php

 session_start();
 require('connection.php'); 

 $sError    = "";
 $sEmail    = "";
 $sPassword = "";

if(isset($_POST['Login'])){

    if(isset($_POST['username'])) $sEmail    = $_POST['username'];
    if(isset($_POST['password'])) $sPassword = $_POST['password'];

    if($sPassword != '' && $sEmail == ''){

        // create an instance of the connection
        $conn   = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );

        // prepare the sql
        $sSQL = "SELECT Email, Id from user_info where email=:email and password = :password ";
        $st   = $conn->prepare( $sSQL );

        // bind the input vars
        $st->bindValue(":email", $sEmail, PDO::PARAM_STR);
        $st->bindValue(":password", $sPassword, PDO::PARAM_STR);
        $st->execute();

        // if data is returned from calling fetch
        if( $row = $st->fetch() ){
            $_SESSION['loggedIn']   = true;
            $_SESSION['email']      = $row['Email'];
            $_SESSION['id']         = $row['Id'];

            header("Location:profile.php");
            exit;

        }

    }else{

        // must have empty input add to errror string
        $sError .= "[EmptyInput]";

    }


    // user hasnt been logged in set error
    $sError .= "[InvalidLogin]";


}



// detect anthign in error stirng
if($sError != ""){

    // do something

}
?>

Havent tested it but should work.

I would also salt and encrypt your passwords.

Additionally I would generate a code to use to identify the user publicly.

How to make PHP input methods(get and post) safe by a simple code?

2 Answers2