3
\$\begingroup\$

I'm using Twitter Bootstrap. I haven't coded anything significant web design related in a while, because I've been doing desktop stuff and games. I'm trying to get back into the flow of things. Can anyone tell me if what I have is sufficient without going into ridiculous detail (the script, not the answer)?

Partial HTML:

<div class='container' id="loginContainer">
<h3>Login</h3>
    <form method="post" name="login" id="login" action="" class="form-horizontal">
        <div class="control-group">
        <label class="control-label" for="inputLoginUsername">Username</label>
        <div class="controls">
        <input type="text" id="inputLoginUsername" name="inputLoginUsername" required placeholder="Username" />
        </div>
        </div>

<div class="control-group">
<label class="control-label" for="inputLoginPassword">Password</label>
<div class="controls">
<input type="password" id="inputLoginPassword" name="inputLoginPassword" required placeholder="Password">
</div>
</div>

    <div class="control-group">
    <div class="controls">
        <button type="submit" name="inputLoginSubmit" id="inputLoginSubmit" class="btn btn-default">Submit</button>
    </div>
    </div>
    </form>
</div>

JavaScript:

$(document).ready(function() {
    $('#register').submit(function(event) {
        event.preventDefault();
        $.post('process.php', {
            inputRegisterSubmit: '',
            inputRegisterUsername: $('#inputRegisterUsername').val(),
            inputRegisterPassword: $('#inputRegisterPassword').val(),
            inputConfirmPassword: $('#inputConfirmPassword').val()
        }).done(function(data) {
            $('#registerOutput').html(data);
            if (data.indexOf("success") !== -1)
                $('#registerContainer').slideUp();
        });
    });

    $('#login').submit(function(event) {
        event.preventDefault();
        $.post('process.php', {
            inputLoginSubmit: '',
            inputLoginUsername: $('#inputLoginUsername').val(),
            inputLoginPassword: $('#inputLoginPassword').val()
        }).done(function(data) {
            $('#loginOutput').html(data);
            if (data.indexOf("success") !== -1)
                $('#loginContainer').slideUp();
        });
    });
});

PHP:

<?php
$mysqli = new mysqli("localhost", "user", "pass", "database");
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: (" . $mysqli->connect_errno . ") " . $mysqli->connect_error;
}

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

    if (!($stmt = $mysqli->prepare("INSERT INTO users(user_name, user_pass, user_email, user_date) VALUES (?, ?, ?, ?)"))) {
        echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
    }


    $user_name = $_POST['inputRegisterUsername'];
    $user_pass = $_POST['inputRegisterPassword'];
    $user_confirm = $_POST['inputConfirmPassword'];
    if ($user_pass != $user_confirm) echo "<p class='text-error'>Your passwords don't match.</p>";
    else {
        $user_email = '[email protected]';
        if (!$stmt->bind_param("ssss", $user_name, $user_pass, $user_email, date("Y-m-d H:i:s"))) {
            echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
        }

        if (!$stmt->execute()) {
            echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
        }
        $stmt->close();
        echo "<p class='text-success'>Log in now!</p>";
    }
} else if (isset($_POST['inputLoginSubmit'])) {

    if (!($stmt = $mysqli->prepare("SELECT user_name FROM users WHERE user_pass = ?"))) {
        echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
    }

    $user_name = $_POST['inputLoginUsername'];
    $user_pass = $_POST['inputLoginPassword'];

    if (!$stmt->bind_param("s", $user_pass)) {
        echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
    }



    if (!$stmt->execute()) {
        echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
    }

    $stmt->store_result();



    if ($stmt->num_rows <= 0)
        echo "<p class='text-error'>Invalid user/pass combination.</p>";
    else
        echo "<p class='text-success'>Logging you in now!</p>";

    $stmt->close();
} else header('Location: http://www.site.com/');

$mysqli->close();
?>
\$\endgroup\$
3
  • \$\begingroup\$ I haven't done anything with PHP in a while but it's good that you are using prepared statements. Are you doing any parameter checking/sanitizing anywhere? \$\endgroup\$ Aug 15, 2013 at 0:43
  • \$\begingroup\$ Yes he is at the bind_param call \$\endgroup\$
    – Pinoniq
    Aug 19, 2013 at 15:36
  • \$\begingroup\$ I recommend you to use tje jquery.form plugin: stackoverflow.com/questions/1960240/jquery-ajax-submit-form \$\endgroup\$
    – Pinoniq
    Aug 19, 2013 at 15:38

1 Answer 1

3
\$\begingroup\$

If you plan on putting this out in the wild (the web), I suggest you handle your error feedback a little more user friendly. If this is for your home server and you'll be the only one using it, it's fine. But displaying binding or executing errors definitely helps attackers get an insight of your methods.

Also, you seem to have forgotten to actually check the username when they log in? You're seeing if there's a row with that password (more on that below), and then if there's one or more accounts with that password, you log them in?

You should be checking to make sure there is exactly one row with the user-submitted username and password. Perhaps you accidentally just forgot this part!

More on your passwords! You have some protection: you're binding. But that's it? You're storing your passwords in plain text! Go ahead and read this to learn a bit more on how you should be keeping your sensitive data in databases!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.