PHP MD5 Create User Form

Question

I have used a tutorial here: http://www.phpeasystep.com/phptu/26.html to create a login form for my website. I have set the uPassword field in my database to be md5 and all of the passwords in the database are encrypted with md5.

The login works perfectly, however I am slightly confused about creating a registration form.

The form requests for a user to input their desired password. I am slightly confused as to how I will then take the password that the user inputs, converting it to md5 and then inputting the md5 password into the uPassword field in the user table.

Below is the code that I have for the processresgistration.php file:

/* Database connection info*/
mysql_select_db("dbname", $con);

$encryptedpassword = md5($_POST['uPassword']);

md5($uPassword);

$sql="INSERT INTO users (uName, uPassword, uSurname, uFirstName)
VALUES
('$_POST[uName]','$encryptedpassword','$_POST[uSurname]','$_POST[uFirstName]'";

if (!mysql_query($sql,$con))
  {
  die('Error: ' . mysql_error());
  }
echo "Account created.  You can now login";

mysql_close($con)
?>

The code above is supposed to:

Create a variable named encryptedpassword
Use uPassword as encryptedpassword
Convert encrypted password to MD5
Input the MD5 password into the users table as uPassword

I'm sure that I've not used a correct variable somewhere, or I have done a simple error with my syntax; any comments/help is greatly appreciated!

Thanks, Chris M

Do you receive any output? There is an error in the first line, add a trailing / to the first line... — powtac, Jan 26 '11 at 10:41
Preventing SQL Injection http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php — acm, Jan 26 '11 at 10:45
Wow, _two_ weaknesses (md5 _and_ unsalted password hash!) and _three_ remotely exploitable [SQL Injection](http://en.wikipedia.org/wiki/SQL_injection) bugs in roughly nine lines of code. That's gotta be some kind of record. I hope. :) — sarnold, Jan 26 '11 at 10:47
the error is in the query closing bracket missing check my answer — Harish, Jan 26 '11 at 11:01
I would strongly advice you against writting your on login system. It is probably going to be very unsafe(I spotted md5,CSRF,SQL injection(Use PDO to prevent this) just to name a few). You should use openid, facebook connect or something like that instead. I really like lightopenid library for its simplicity => http://gitorious.org/lightopenid — Alfred, Jan 26 '11 at 11:30

score 0 · Answer 1 · answered Jan 26 '11 at 10:43

0

/* Database connection info*

You didn't properly close your comment there. Add a / at the end of the line.

Oh, and MD5 is insecure. Use SHA1 instead. Or even better, use salted SHA1.

You also need to start escaping all user input you are putting in your database using mysql_real_escape_string() or Little Bobby Tables will have lots of fun with your database.

answered Jan 26 '11 at 10:43

ThiefMaster

310,957
84
592
636

nothing bad with MD5 and "escaping all user input" is a nonsense. and bobby's fun with PHP's mysql library is very limited. – Your Common Sense Jan 26 '11 at 11:08
1

escaping user input is a nonsense. one should escape not "all" "input" but *string literals* going to the *query*. The destination matters, not source. – Your Common Sense Jan 26 '11 at 11:12
That's obviously what I've meant. ;x – ThiefMaster Jan 26 '11 at 12:16

Harish · Answer 2 · 2011-01-26T14:56:07.840

0

/* Database connection info*/

 mysql_select_db("dbname", $con);

$encryptedpassword = md5($_POST['uPassword']);

$sql="INSERT INTO users (uName, uPassword, uSurname, uFirstName)
VALUES
('$_POST[uName]','$encryptedpassword','$_POST[uSurname]','$_POST[uFirstName]')";

if (!mysql_query($sql,$con))
  {
  die('Error: ' . mysql_error());
  }
echo "Account created.  You can now login";

mysql_close($con)
?>

The closing of bracket is the issue in query

('$_POST[uName]','$encryptedpassword','$_POST[uSurname]','$_POST[uFirstName]'";

This needs a closing ' ) '

edited Jan 26 '11 at 14:56

answered Jan 26 '11 at 10:58

Harish

2,311
4
23
28

`md5($uPassword);` - this line is useless – ThiefMaster Jan 26 '11 at 14:53
i just tried him to show missing brackets in his code okey removed that – Harish Jan 26 '11 at 14:55

score 0 · Accepted Answer · answered Jan 26 '11 at 10:58

You code require some validation & escaping, more like this :

<?php

/* Database connection info */
mysql_select_db("dbname", $con);

if ($_REQUEST['METHOD'] == 'POST') {
    $uName = filter_input(INPUT_POST, 'uName');
    $uPassword = filter_input(INPUT_POST, 'uPassword');
    $uSurname = filter_input(INPUT_POST, 'uSurname');
    $uFirstName = filter_input(INPUT_POST, 'uFirstName');

    // do some validation here ...

    // if everything OK, then crypte the password
    $hashedPassword = md5($uPassword);

    // and store it

    $sql = sprintf(
        'INSERT INTO users (uName, hashedPassword, uSurname, uFirstName)
         VALUES (%s, %s, %s, %s);',
            mysql_real_escape_string($uName, $con),
            mysql_real_escape_string($hashedPassword, $con),
            mysql_real_escape_string($uSurname, $con),
            mysql_real_escape_string($uFirstName, $con)
    );

    if (!mysql_query($sql,$con)) {
        die('Error: ' . mysql_error());
    }

    mysql_close($con);
    echo "Account created.  You can now login";
}

?>

Now for login

<?php

if ($_REQUEST['METHOD'] == 'POST') {
    $uName = filter_input(INPUT_POST, 'uName');
    $uPassword = filter_input(INPUT_POST, 'uPassword');
    $hashedPassword = md5($uPassword);

    $sql = sprintf(
        'SELECT * FROM users WHERE uName = "%s" AND hashedPassword = "%s" LIMIT 1',
            mysql_real_escape_string($uName, $con),
            mysql_real_escape_string($hashedPassword, $con),
        );

    // etc etc ...
}
?>

I'd suggest to add password salting to this code, just using other user's attributes. and, of course, `trigger_error` instead of die — Your Common Sense, Jan 26 '11 at 11:18

PHP MD5 Create User Form

3 Answers3