-1

I got the following code and nothing is getting inserted into my database and I can't figure out why.. is the password_needs_rehash() function at it's right spot?

I got a test where I echo out the new generated with a Javascript alert and it shows a new hash but the new hash is not getting inserted into the database. There must be a error in my logic but I can't find it.

<?php
session_start();
include 'mysqli.php';


$userID = strtolower($_POST['userID']);
$passwordFromForm = $_POST['password'];
$_SESSION['userID'] = $userID;

//Select the password from the requested userID and put into variable
$sql = "SELECT password FROM userdata WHERE userID = '$userID'";
$result = mysqli_query($conn, $sql);
$row = mysqli_fetch_assoc($result);
$passwordFromDatabase = $row['password'];

$verified = password_verify($passwordFromForm, $passwordFromDatabase);

if($verified == 0){
echo '<meta http-equiv="refresh" content="0; URL=index.php?triedLogin" />';

}else{

if(password_needs_rehash($passwordFromDatabase, PASSWORD_DEFAULT, ['cost' => 14])){

    $passwordFromDatabase = password_hash($passwordFromDatabase, PASSWORD_DEFAULT, ['cost' => 14]);

    echo "<script>alert('".$passwordFromDatabase."');</script>";

    $sql = "INSERT INTO userdata ('password') VALUES ('$passwordFromDatabase')";

    //NOW LOGIN WITH NEW PASSWORD
    $sql = "SELECT * FROM userdata WHERE userID = '$userID' AND password = '$passwordFromDatabase'";
    $result = mysqli_query($conn, $sql);

        if(!$row=mysqli_fetch_assoc($result)){
            echo '<meta http-equiv="refresh" content="0; URL=index.php?triedLogin" />';
        } else {
            $_SESSION['ID'] = $row['userID'];
            unset($_SESSION['userID']);
            echo '<meta http-equiv="refresh" content="0; URL=app/index.php" />';
        }

}else{

    $sql = "SELECT * FROM userdata WHERE userID = '$userID' AND password = '$passwordFromDatabase'";
    $result = mysqli_query($conn, $sql);

    if(!$row=mysqli_fetch_assoc($result)){
        echo '<meta http-equiv="refresh" content="0; URL=index.php?triedLogin" />';
    } else {
        $_SESSION['ID'] = $row['userID'];
        unset($_SESSION['userID']);
        echo '<meta http-equiv="refresh" content="0; URL=app/index.php" />';
    }
}
}
?>
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Zoung
  • 43
  • 7
  • This `INSERT INTO userdata ('password')` for one thing, is failing you, plus you never executed that query. – Funk Forty Niner Nov 05 '16 at 00:55
  • ...you are paying attention here, right? Probably not. Should I post an answer to get your attention instead? – Funk Forty Niner Nov 05 '16 at 00:56
  • *"is the rehash_password function at it's right spot?"* - Huh, what `rehash_password()` function? – Funk Forty Niner Nov 05 '16 at 01:00
  • but shouldn the insert request be executed when stated in a variable? I also saw that i had the sql twice now :O @Fred-ii- – Zoung Nov 05 '16 at 01:01
  • *password_needs_rehash sorry, I'm tired haha – Zoung Nov 05 '16 at 01:02
  • I take it your insert still doesn't work then. Ahmed's answer is really a glorified comment and not an answer/solution to this question, IMHO. We also don't know if your POST arrays contain values or not and what they are exactly. – Funk Forty Niner Nov 05 '16 at 01:05
  • the $_POST['userID'] & $_POST['password'] are the plain text coming from a form to login @Fred-ii- I wrote this code now instead to somehow query the insert but nothing is getting inserted here either :( `$newEntry = "INSERT INTO 'userdata' ('password') VALUES ('$passwordFromDatabase')"; $getgoing = mysqli_query($conn, $newEntry);` – Zoung Nov 05 '16 at 01:08
  • **WARNING**: When using `mysqli` you should be using [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use string interpolation or concatenation to accomplish this because you have created a severe [SQL injection bug](http://bobby-tables.com/). **NEVER** put `$_POST` or `$_GET` data directly into a query, it can be very harmful if someone seeks to exploit your mistake. – tadman Nov 05 '16 at 01:54
  • **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. – tadman Nov 05 '16 at 01:57
  • One more thing: Don't use the password from the database. If you've verified this is the correct password, use the password supplied by the user. You might have a hashed password with something like MD5 originally, so this is a better pattern to employ in a general sense. – tadman Nov 05 '16 at 01:58
  • thanks for the hint @tadman I'm not going to keep this structure, this is just in development and i will rewrite the whole mysql bit into pdo before any release ;) – Zoung Nov 05 '16 at 02:01
  • I'd strongly advise you to take this opportunity to evaluate existing frameworks which implement this correctly and make full use of them. This stuff is very, very hard to get right. – tadman Nov 05 '16 at 02:02
  • I deleted my answer after all. I felt insulted seeing the other answer getting an upvote and not mine and didn't even cover a fraction of the problems with this question, what with the work I put into this. I knew I wouldn't see the end of this tunnel. – Funk Forty Niner Nov 05 '16 at 02:24

1 Answers1

0
  • Make make sure your table field is large enough to hold that value because hash are very long, it will throw errors thus no INSERT.

  • You should prepare your statements, I am surprised you don't do this yet you into password hashing.

meda
  • 45,103
  • 14
  • 92
  • 122
  • Eh... you never run the `$sql` with the insert. You overwrite it with the `$sql` with the select right below it. – junkfoodjunkie Nov 05 '16 at 00:54
  • My password field is of the TEXT typ so it should fit the hashed string. I will prepare all the mysql inserts etc when the webapp is close to finished ;) I only follow others guides and the most available help is on the mysqli front, unfortunately. I will however rewrite everything later on before releasing ;) @Ahmed Daou – Zoung Nov 05 '16 at 00:56
  • Thanks!!! I thought the first sql gets executed and then moves on to the next variable hehe @junkfoodjunkie – Zoung Nov 05 '16 at 00:57
  • @Zoung Using `TEXT` for short strings is a complete waste. Use `VARCHAR(255)` for all generic strings, `TEXT` for anything that's >1KB, which a hashed password is obviously not. – tadman Nov 05 '16 at 01:55