-1
<html>
<body>

<div style="margin:0 auto" align=center>
<form>
  Username:<br>
  <input type="text" name="username"><br>
  Current Password:<br>
  <input type="password" name="password"><br>
  New Password:<br>
  <input type="password" name="newpassword"><br>
  Confirm Password:<br>
  <input type="password" name="confirmnewpassword"><br>
  <br>
  <input type="submit" name="submit" value="Submit" />
</form>

<?php

$dbhost = "*****";
$dbname = "*****";
$dbuser = "*****";
$dbpass = "*****";
//Connect to database

$connect= mysql_connect ("$dbhost","$dbuser","$dbpass")or die("Could not connect: ".mysql_error());
mysql_select_db("$dbname") or die(mysql_error());

if(isset($_POST['submit']))

    $username = $_POST['username'];
    $password = md5($_POST['password']);
    $newpassword = md5($_POST['newpassword']);
    $confirmnewpassword = md5($_POST['confirmnewpassword']);

    $result = mysql_query("SELECT password FROM accounts WHERE username='$username'");

    $row = mysql_fetch_assoc($result);

    $passworddb = $row['passoword']; //password from Data Base
    if(!$result) {
        echo "The username you entered does not exist";
    }
    if($password==$passworddb){
        if($newpassword==$confirmnewpassword){
            $sql=mysql_query("UPDATE accounts SET password='$newpassword' where username='$username'");
?> 
<script>
    alert('Password changed!');
    window.location.href='change_password.php';
</script> 
<?php
        }
        else{
?> 
<script>
    alert('Error, new password and confirm password must be the same');
    window.location.href='change_password.php';
</script> 
<?php
    }
}
?>
</body>
</html>

Can any one tell me what is wrong in my codes please? This is my change password page, it contains my MySQL connect. The password change form. When I press submit, nothing happens so can any one tell me what is wrong with my code please?

RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
  • 2
    Please dont use [the `mysql_` database extension](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php), it is deprecated (gone for ever in PHP7) Specially if you are just learning PHP, spend your energies learning the `PDO` database extensions. [Start here](http://php.net/manual/en/book.pdo.php) its really pretty easy – RiggsFolly Jul 20 '16 at 16:50
  • 2
    Please dont __roll your own__ password hashing. PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them, I might want to use your site one day And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) – RiggsFolly Jul 20 '16 at 16:50
  • 2
    Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared statement and parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Jul 20 '16 at 16:51
  • 1
    Some sensible code indentation would be a good idea. It help us read the code and more importantly it will help **you debug your code** [Take a quick look at a coding standard](http://www.php-fig.org/psr/psr-2/) for your own benefit. You may be asked to amend this code in a few weeks/months and you will thank me in the end. – RiggsFolly Jul 20 '16 at 16:52
  • thanks for respond, but i just want to know what is wrong with my codes then i will learn pdo – Medo Mohamed Jul 20 '16 at 16:53
  • 1
    ***You really shouldn't use [MD5 password hashes](http://security.stackexchange.com/questions/19906/is-md5-considered-insecure)*** and you really should use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. 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 Jul 20 '16 at 16:55
  • 1
    Oh yea right, we believe him dont we @JayBlanchard! – RiggsFolly Jul 20 '16 at 16:56
  • 1
    I hate when people say *"I'm not that far along..."* or *"This site will not be public..."* or *"It's only for school, so security doesn't matter..."*. If teachers and professors are not talking about security from day one, they're doing it wrong. Challenge them. They're teaching sloppy and dangerous coding practices which students will have to unlearn later. I also hate it when folks say, *"I'll add security later..."* or *"Security isn't important now..."* or *"Ignore the security risk..."*. If you don't have time to do it right the first time, when will you find the time to add it later? – Jay Blanchard Jul 20 '16 at 16:56
  • 10-4 @RiggsFolly. BTW, nothing from our Canadian friend. I thought they'd be back by now. – Jay Blanchard Jul 20 '16 at 16:56
  • @JayBlanchard Didnt realise they were away – RiggsFolly Jul 20 '16 at 16:57
  • A query to select password will not fail if the userid does not exist, it will however return an empty result set. That is not the same as `$result` equalling false – RiggsFolly Jul 20 '16 at 16:58
  • With that code, any user could change any other users password just by knowing his/her username... Make sure, that you select or update the account only, if the old password was correct. – Honk der Hase Jul 20 '16 at 17:29

2 Answers2

1

Don't know if you noticed this but:

$passworddb = $row['passoword']; //Here may be your error

It should match your sql statement

$passworddb = $row['password'];

You are passing your variables through the URL. You need to $_GET for your variable to be received on the php page:

if(isset($_GET['submit']))

    $username = $_GET['username'];
    $password = md5($_GET['password']);
    $newpassword = md5($_GET['newpassword']);
$confirmnewpassword = md5($_GET['confirmnewpassword']);

$result = mysql_query("SELECT password FROM accounts WHERE username='$username'");

$row = mysql_fetch_assoc($result);

$passworddb = $row['passoword']; //password from Data Base
if(!$result) {
    echo "The username you entered does not exist";
}
if($password==$passworddb){
    if($newpassword==$confirmnewpassword){
        $sql=mysql_query("UPDATE accounts SET password='$newpassword' where username='$username'");
?> 

But this is a bad way to pass the account data over as it will open you up to attacks

WorkHardWork
  • 92
  • 10
  • no and this is site roleplayucp.pe.hu this is my forum test it, username kevin_kevin, current pass : 123456 – Medo Mohamed Jul 20 '16 at 17:15
  • you are using $_POST here :`if(isset($_POST['submit']))` change to `if(isset($_GET['submit']))` you are passing the value in the URL so it's not a `POST` call. It'll be the same for all your `POST` varibles – WorkHardWork Jul 20 '16 at 17:21
0

$passworddb = $row['**passoword**']; //password from Data Base

you have a misspelling here

Radu Radu
  • 177
  • 16