0

I'll explain the code below : I'm submitting a written password inside the form where it is taken, and it updates the current password that already exists for the user (where the e-mail is the e-mail that is already there from his session, if that makes sense).

So basically if the the SQL query works (if($result)), it should redirect us to a certain page, else it redirects us to the homepage. So it always redirects us to the homepage, thus I believe the query is wrong, but I can't figure it out. Any ideas?

<?
include("config.php");
session_start();

   if($_SERVER["REQUEST_METHOD"] == "POST") {
      // password sent from form 

      $mypassword = mysqli_real_escape_string($conn,$_POST['password']); 


      $sql = "UPDATE Users SET password='$mypassword' WHERE email = {$_SESSION['email']}";
      $result = mysqli_query($conn,$sql);

      // If result matched $myemail and $mypassword, table row must be 1 row

      if($result) {
    $_SESSION['password']=$mypassword;

    header("location: logged_in.php");

      } else {
        header("location: index.php");
      }
   }
?>

<?php include('head.php'); ?>
<?php include('nav.php'); ?>

    <form class="form-signin" role="form" method="post" action="changepass.php">    

      <h2 class="form-signin-heading">Change password</h2>
      <p>Please change your password.</p>

      <input type="password" class="form-control" name="password" placeholder="Password" required=""/>

      <button class="btn btn-lg btn-primary btn-block" type="submit">Changer le mot de passe</button>   

    </form>

<?php include('footer.php'); ?>
  • 2
    Please tell me you're not storing passwords in plain text – Darren H Oct 12 '16 at 20:57
  • 1
    Plain passwords, cool – u_mulder Oct 12 '16 at 20:57
  • I am, please explain why is it wrong and how should I do it otherwise? It doesn't reply my question but you got me curious. @DarrenH –  Oct 12 '16 at 20:58
  • 2
    the query is failing that's why the redirect is falling, the query is probably failing due to the lack of quotes around the string email address –  Oct 12 '16 at 20:59
  • 2
    There a numerous articles about why you should **never** store passwords in plain text. https://www.owasp.org/index.php/Password_Plaintext_Storage – chris85 Oct 12 '16 at 21:00
  • @nogad lack of quotes around which email adress string? {$_SESSION['email']} ? There are quotes tho in there.... –  Oct 12 '16 at 21:00
  • You need to check if the `mysqli_query` call succeeded or failed. Use `mysql_error` to get any error message. The docs have an example of error handling MySQL: http://php.net/mysqli_error – leepowers Oct 12 '16 at 21:00
  • 2
    **Never store plain text passwords!** Please use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). Make sure you ***[don't escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Oct 12 '16 at 21:01
  • 2
    From @tadman: 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. – Jay Blanchard Oct 12 '16 at 21:01
  • Please google or search this site for reasons not to store plain text passwords. I'd expect the number of articles on the subject to be in the tens of thousands as a conservative estimate – Darren H Oct 12 '16 at 21:01
  • Also, you should use `mysql_prepare` to property escape and bind parameters (like email) to SQL queries: http://php.net/manual/en/mysqli.prepare.php – leepowers Oct 12 '16 at 21:02
  • quotes here: `where x="y"` –  Oct 12 '16 at 21:02
  • 2
    [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)*** Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! [Don't believe it?](http://stackoverflow.com/q/38297105/1011527) – Jay Blanchard Oct 12 '16 at 21:03
  • 1
    You should `exit` after the `header`s and use parameterized queries. – chris85 Oct 12 '16 at 21:03
  • 1
    You didn't quote the email address, producing `... email=president@whitehouse.gov`, causing a "no such field" syntax error. Since your code simply ASSUMES nothing could ever go wrong, you didn't check for failure, and propagated the failure onwards. – Marc B Oct 12 '16 at 21:04

0 Answers0