0

Good afternoon.

I'm a beginner in PHP programming, followed some courses and have a theoretical knowledge about it. I've been hired now and been given some 'basic' tasks. Coworkers here tell me that 'real world' code differs a bit from what it is taught at University, in books and the like. I've been reading about security and found out about SQL-injection. I also learned that the best way to avoid them is using prepared statements and parameters bounding.

So, I'd be very thankful if you could give me your opinions or suggestions about the code below. Please, anything you have to say will be very useful. Opinions about the logic of the code, about the structure, about performance, about security... anything.

if (isset($_POST['username']) && isset($_POST['password']))
{
    $dbaddress = 'myhost';
    $dbuname = "database_user";
    $dbpass = "database_password";
    $dbname = 'customers_db';

    $r = new mysqli($dbaddress, $dbuname, $dbpass, $dbname);

    $q = $r->prepare("SELECT * FROM users WHERE uAccessName = ? AND uAccessPass = ?");
    $q->bind_param("ss", $user, $pass);

    $user = $_POST['username'];
    $pass = $_POST['password'];
    $q->execute();
    $q->store_result();

    if ($q->num_rows === 1)
        // Do many many other things here
        echo "<h1>Access granted</h1>";
    else
        echo "<h1>Access denied</h1>";

    $q->close();
    $r->close();
} else {
    // Handle the case where the form sent no data
}

Thanks a lot.

4
  • 4
    Probably better if you posted this on code review: codereview.stackexchange.com Commented Jan 18, 2017 at 18:31
  • @Tom I hope you understood and I given my best answer Commented Jan 18, 2017 at 18:50
  • If we're talking about "efficient" then putting $_POST data into bind_param without intermediate variables is more efficient. The good news is you're using prepared statements, that's a big step towards writing secure code. If you want to learn more, worth reading up on what OWASP has to offer. Commented Jan 18, 2017 at 19:04
  • 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 like Laravel comes with a robust authentication system built-in. At the absolute least follow recommended security best practices and never store passwords as plain-text. Commented Jan 18, 2017 at 19:04

1 Answer 1

-1

Do not store the database passwords in plain text. Use a two way encryption algorithm so you can store the database password in encrypted form, but you can still use it in database queries

Also sanitize the POST variables using filter_var or similar Php function.

Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.