-3

I'm a bit new to answering questions, so bear with me here. And i've read posts, and i've seen that mysql_ is not being used anymore. I know that, so don't tell me not to use it.

Anyhow, I've got this code, and whenever I try to sign up using it, the password isn't hashed in MD5. I've removed the

$mdhash = md5('$name');

because I want your help.

Here is the code:

<?php

    session_start();

    include('./dbconnect/global.php');

    if (isset ($_POST['submit'])) {
    $comment = mysql_escape_string (trim ($_POST['username']));
    $id = 0;
    $date = date("Y-m-d");
        if(!empty($_POST['username']) && !empty($_POST['password']) && !empty($_POST['email'])){
            if ($_POST['username']=="telamon" or $_POST['username']=="brick") {
                die("This username is banned");
            }
                $name = mysql_escape_string (trim ($_POST['password']));
                $mdehash = md5($name);
                $email = mysql_escape_string (trim ($_POST['email']));
                $picture = $_POST['def'];
                $tweet = 'NoTweetsTweeted';
                $g = mysql_query( "SELECT * FROM admin WHERE username='".$comment."'") or die(mysql_error());

                if (mysql_num_rows($g) >= 1) {
                    $errMsg = "<p style='color:#999999;'>That username is already registerd!</p>";
                }else {
                    if ( preg_match("/^[a-zA-Z0-9]+$/i", $comment) ) { 
                    $sql = mysql_query ("INSERT INTO admin (id,username,password,date,picture,email,twitter) VALUES ('".$id."','".$comment."','".$name."','".$date."','".$picture."','".$email."','".$tweet."')");
                    header('location: home.php');
                    }else
                    $errMsg = "<p style='color:#999999;'>Please fill all fields!</p>";
            }
        }
    }
?>

So, if I could get some help with this, that'd be greatly appreciated! Thanks in advance!

Preet Sangha
  • 64,563
  • 18
  • 145
  • 216
  • 4
    Don't use md5 but a salted hash using a more secure algorithm instead. Also you **must** use `mysql_real_escape_string` instead of `mysql_escape_string` - or even better, **use PDO or mysqli** instead of the deprecated mysql api. – ThiefMaster Nov 16 '12 at 01:38
  • 3
    What _is_ your question? – Matt Ball Nov 16 '12 at 01:38
  • Where are you using `$mdehash`? You define it as `md5($name)` but later use `$name` in your `INSERT` statement directly. – Michael Berkowski Nov 16 '12 at 01:39
  • **tl;dr;** *use an existing library*. This isn't a new problem, and many people get it *very wrong*. It's very easy to get security incorrect, especially when *not* really understanding how it works. There are *many* other duplicates showing this. –  Nov 16 '12 at 01:40
  • possible duplicate of [Difference between single quote and double quote string in php](http://stackoverflow.com/questions/3446216/difference-between-single-quote-and-double-quote-string-in-php) – mario Nov 16 '12 at 01:43

2 Answers2

5

Get rid of the single quote in this statement.

$mdhash = md5('$name');

It should be:

$mdhash = md5($name);

The single quotes cause the dollar sign to be taken literally and as a result $name is not a variable anymore but a literal string.

John Conde
  • 217,595
  • 99
  • 455
  • 496
  • you could also mention why `md5("$name")` would work but be bad; i think it'd somewhat fit into the answer to his question. the asker seems to be a beginner and he'll likely stumble upon that kind of "code" – ThiefMaster Nov 16 '12 at 01:39
  • Actually I saw your comment and figured that covered it. Hopefully when PHP5.5 comes out everyone will ditch the old tutorials and start using the new password API. – John Conde Nov 16 '12 at 01:40
  • @JohnConde Considering people still "suggest" the hackish `mysql_real_escape_string` (or ignore SQL injection issues entirely) .. I think not. Too much bad junk on/in PHP .. –  Nov 16 '12 at 01:41
  • 3
    You also still believe in santa, don't you? :) I don't think all the crap out there will ever disappear or finally end up not being used. If PHP decides to ditch all the crap (mainly the mysql extension) you'll just see new questions here how to fix that "unknown function" error... – ThiefMaster Nov 16 '12 at 01:42
0

It looks as though you're passing $name into the SQL Database instead of $mdehash in your query.

I'd also change the variable names to better match their actual usage. They're slightly confusing.

BrenanK
  • 667
  • 5
  • 4