-5

I'm using code that I've used on another webpage, which works fine on the other page, but not on this new one. The MySQLi table for logins is setup exactly the same too. I don't think it's the login script, because I inserted my password hash from the other site into the database to test it, and I can login with it.

This is the registration code:

include('sql.php');
$username = $_POST['username'];
$password = $_POST['password'];
$confirm = $_POST['confirm'];

if($username == '' || $password = '') {
    header('Location:/register.php');
}
if($password != $confirm) {
    header('Location:/register.php');
}
$sql = "INSERT INTO login (username, password) VALUES ('" . $username . "', '" . password_hash($password, PASSWORD_DEFAULT) . "')";
if(mysqli_query($mysqli, $sql)) {
    header('Location: /dashboard.php');
} else {
    echo $mysqli->error;
}
mysqli_close($mysqli);

And the login:

session_start();
$error = '';

if(isset($_POST['submit'])) {
    if(empty($_POST['username']) || empty($_POST['password'])) {
        header("Location: /admin.php?error=invalid");
    } else {
        include('sql.php');
        $username = mysqli_real_escape_string($mysqli, stripslashes($_POST['username']));
        $password = mysqli_real_escape_string($mysqli, stripslashes($_POST['password']));
        $sql = "SELECT * FROM login WHERE username='" . $username . "'";
        $result = $mysqli->query($sql);
        if($result->num_rows == 1) {
            while($row = $result->fetch_assoc()) {
                $verify = password_verify($password, $row['password']);
                if($verify == false) {
                    header("Location: /admin.php?error=mismatch");
                } else {
                    $_SESSION['login_user'] = $username;
                    $_SESSION['login_pass'] = $password;
                    if($_POST['stay'] == 'stay') {
                        setcookie('username', $username, time() + 31536000, '/');
                        setcookie('password', $password, time() + 31536000, '/');
                    }
                    header("location: /dashboard.php");
                }
            }
        } else {
            header("Location: /admin.php?error=mismatch");
        }
        mysqli_close($mysqli);
    }
}
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
jcvamp
  • 25
  • 8
  • [Little Bobby](http://bobby-tables.com/) says **[you are at risk for SQL Injection Attacks](https://stackoverflow.com/q/60174/)**. Learn about [Prepared Statements](https://en.wikipedia.org/wiki/Prepared_statement) for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even **[escaping the string](https://stackoverflow.com/q/5741187)** is not safe! I recommend `PDO`, which I [wrote a function for](https://stackoverflow.com/a/45514591) to make it extremely **easy**, very **clean**, and way more **secure** than using non-parameterized queries. – GrumpyCrouton Aug 10 '17 at 20:42
  • @PatrickQ yes, they do. – jcvamp Aug 10 '17 at 21:15

1 Answers1

4

Well, this shouldn't have been as hard to spot as it was.

if($username=='' || $password=''){header('Location:/register.php');}

Specifically $password=''

That's an assignment, not an equality check. Change it to $password=='' and I bet it works.

Patrick Q
  • 6,373
  • 2
  • 25
  • 34
  • Good spot. Deceived by it, and theoretically, the question should have been closed as a duplicate of [The 3 different equals](https://stackoverflow.com/questions/2063480/the-3-different-equals) – Funk Forty Niner Aug 10 '17 at 21:27
  • I'm just wondering if someone's going to downvote you for not rewriting it as a prepared statement or as they said; leaving them open to an sql injection. *Sigh* – Funk Forty Niner Aug 10 '17 at 21:30
  • @Fred-ii- lol. I did fight the urge to add some spaces in there, as that's my personal preference :) Also, I wouldn't necessarily object to this question being closed. Just wanted to make sure the OP actually saw where the error was. – Patrick Q Aug 10 '17 at 21:32
  • *Aye*, true. After all this time of staring at it and commenting, you deserve it, the upvotes/acceptance that is *lol!* not dv's. – Funk Forty Niner Aug 10 '17 at 21:32