SitePoint Sponsor

User Tag List

Results 1 to 25 of 38

Hybrid View

  1. #1
    SitePoint Member
    Join Date
    Sep 2005
    Location
    Belgium
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Criticize and correct my login script please

    Edited code [7/1/2006 - evening]
    Edited code [8/1/2006 - 17:04]
    Edited code [9/1/2006 - 20:29]

    Good day,

    some time ago it seemed interesting to learn a dynamic web application language, because I was still making all my sites in HTML. I first tried ASP, but when I almost got the hang of it I suddenly noticed I could have been better learning ASP.NET (with Microsoft's NET framework). Anyway, I choosed PHP then, mainly because it looked more simple, and you didn't have to choose another language like c# or vb to program the application in.

    Fortunately I'm already experienced with some other programming languages, and it was quite simple. In a weekend I learnt php and rewrote the site of my mmorpg (from 70+ files to one index.php (MySQL)).

    But that was quite easy, it was just some MySQL managing, that's it. Anyway, now I have to make a login script with sessions for a site of a game clan. I'm capable of doing that, but the problem is the security, I'm not experienced with that. That's why I want you guys to check and correct my script, it can be possible it have to be rewritten totally .

    Anyway, here it is:

    [Note]: Unfortunately there is no other 'encryption' used except from md5 (that's actually a hashing algorithm). I noticed PHP only has the crypt function as standard, and unfortunately I found out that's also just a hashing script. As you can see I tried some with mcrypt, a seperated library, but unfortunately that lib is not standard and also not supported by my host.

    [Note II]: You will also notice there is no DOCTYPE at the beginning of the html pages. Anyway, when I got my php login script ready I will rewrite it to xhtml and add a DOCTYPE.

    [Note III]: Some html code is not used correctly, for example I've written <br> instead of <br /> a few times. Well, I just want to say I'm aware of that, I'll improve my code when I got my final login script.

    [Note IV, I made this red because I think I really did this wrong.]: Notice my way of securing. What I do is this: when he logs in and it's correct I store three session variables. One md5 hash of the username + hashed password, and two variables containing the username (not hashed because I will need it later) and the password hashed. When he enters another page he takes the two session variables with the username and hashed password, makes a new hash of it and compares it to the hash that was already stored when he logged in.

    Offline

    Now you've read this script you will probably say, "What the hell is that for a way of securing", and I know, it's a stupid way and certainly not secure. Anyway, that's why I asked you guys to help me.

    I also read some about the mysql_real_escape_string() function, about html or mysql injection. Anyway, it was all quite unclear for me so I didn't add it to my script yet. Is that needed? What does it do exactly? It replaces some certain characters so injection is not possible? Something like that?
    Last edited by Xargo; Jan 20, 2006 at 13:41.

  2. #2
    SitePoint Enthusiast duckax's Avatar
    Join Date
    Aug 2005
    Posts
    94
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You might want to check out session_regenerate_id(). Gives the user a new session id before login to make sure that the current one is not planted by some hacker.
    http://www.php.net/session_regenerate_id

    mysql_real_escape_string() is usually not needed in a login script. However, it will be needed when you let your user enter their password, username, etc into your database. Simply escape everything that you put into your DB.

    You will also need to know about the evil magic qoutes.
    http://www.webmasterstop.com/63.html

    Hope this helps.

  3. #3
    SitePoint Member
    Join Date
    Sep 2005
    Location
    Belgium
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by duckax
    You might want to check out session_regenerate_id(). Gives the user a new session id before login to make sure that the current one is not planted by some hacker.
    http://www.php.net/session_regenerate_id

    mysql_real_escape_string() is usually not needed in a login script. However, it will be needed when you let your user enter their password, username, etc into your database. Simply escape everything that you put into your DB.

    You will also need to know about the evil magic qoutes.
    http://www.webmasterstop.com/63.html

    Hope this helps.
    Great, I will add session_regenerate_id() to the code of my first post. Please say it if I add it wrong.

    I don't know if I need something with the magic quotes. I always read much about it, but actually I never ever got problems with those errors by not using these functions with the quotes etc. And I only allow the users to use letters and ciphers anyway. Or isn't that a good idea for a password?

    I also added another note to the first post.

  4. #4
    SitePoint Zealot
    Join Date
    Aug 2005
    Location
    South Africa
    Posts
    185
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I am not sure if you have magic quotes on or off but I do notice that $user is not validated,

    An extreme example, please bear with me

    if I make $user in the form

    0";delete from go_admins where 1="1

    the resulting query becomes

    SELECT pass FROM go_admins WHERE user="0";delete from go_admins where 1="1"

    not really what you wanted.

    You might also want to consider creating a Hashed Message Authentication Code for the password using your own key aswell. Something like the following would be be better and more secure:

    PHP Code:
    function hmacMD5($data$key)
        {
            
    // HMAC(Data) = Hash(SecretKey, Hash(SecretKey, Data))
            
    $block_size 64// byte length for md5
            
    if ( strlen($key) > $block_size )
                
    $key pack("H*"md5($key));
            
    // Fill the rest of the key with NULL if shorter than block_size
            
    $key  str_pad($key$block_sizechr(0x00));
            
    // Create some padded vars to Xor with our key
            
    $ipad str_pad(''$block_sizechr(0x36));
            
    $opad str_pad(''$block_sizechr(0x5C));
            
    $k_ipad $ipad $key;
            
    $k_opad $opad $key;
            return 
    md5($k_opad  pack("H*"md5($k_ipad $data)));
        }

    $encryptedPassword hmacMD5('mypassword''mySeCret'); 
    You might also consider this to be a little over the top

    --
    lv

  5. #5
    SitePoint Member
    Join Date
    Sep 2005
    Location
    Belgium
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lvismer
    I am not sure if you have magic quotes on or off but I do notice that $user is not validated,

    An extreme example, please bear with me

    if I make $user in the form

    0";delete from go_admins where 1="1

    the resulting query becomes

    SELECT pass FROM go_admins WHERE user="0";delete from go_admins where 1="1"

    not really what you wanted.
    Omg damn, I'm so glad you saw that. Never thought about that. How can I avoid that? Make a string length limit? Or not allow any characters except from numbers and letters? Or maybe better, just put the username between to "'s in the MySQL query?

    Quote Originally Posted by lvismer
    You might also want to consider creating a Hashed Message Authentication Code for the password using your own key aswell. Something like the following would be be better and more secure:

    PHP Code:
    function hmacMD5($data$key)
        {
            
    // HMAC(Data) = Hash(SecretKey, Hash(SecretKey, Data))
            
    $block_size 64// byte length for md5
            
    if ( strlen($key) > $block_size )
                
    $key pack("H*"md5($key));
            
    // Fill the rest of the key with NULL if shorter than block_size
            
    $key  str_pad($key$block_sizechr(0x00));
            
    // Create some padded vars to Xor with our key
            
    $ipad str_pad(''$block_sizechr(0x36));
            
    $opad str_pad(''$block_sizechr(0x5C));
            
    $k_ipad $ipad $key;
            
    $k_opad $opad $key;
            return 
    md5($k_opad  pack("H*"md5($k_ipad $data)));
        }

    $encryptedPassword hmacMD5('mypassword''mySeCret'); 
    You might also consider this to be a little over the top

    --
    lv
    Hmmmm.... Has it any use to make a function that hashes using md5 with a key? Md5 is not reversable anyway.

  6. #6
    SitePoint Zealot
    Join Date
    Aug 2005
    Location
    South Africa
    Posts
    185
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Xargo
    Hmmmm.... Has it any use to make a function that hashes using md5 with a key? Md5 is not reversable anyway.
    If for some reason the session is highjacked one can use brute force to hack away at the password as with MD5 alone only data integrity is assured.

    A MAC verifies the authenticity and the integrity of the data. Basically the hmacMD5 function creates a hashed message authentication code (HMAC) which is more than just using md5 with a key. A digest algorithm (like md5) only guarentees the integrity. If someone manages to alter the current MD5 hash with their own md5 version you would not be able to catch that unless you check the username and password combination every time.

    As an example, storing the username and an HMAC of the username in a session one would not need to store the password in the session variable at all.

    --
    lv

  7. #7
    SitePoint Member
    Join Date
    Sep 2005
    Location
    Belgium
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lvismer
    If for some reason the session is highjacked one can use brute force to hack away at the password as with MD5 alone only data integrity is assured.

    A MAC verifies the authenticity and the integrity of the data. Basically the hmacMD5 function creates a hashed message authentication code (HMAC) which is more than just using md5 with a key. A digest algorithm (like md5) only guarentees the integrity. If someone manages to alter the current MD5 hash with their own md5 version you would not be able to catch that unless you check the username and password combination every time.

    As an example, storing the username and an HMAC of the username in a session one would not need to store the password in the session variable at all.

    --
    lv
    So if I understand it right I only have to store the username and the HMAC of the password in the session? No other md5 hashes anymore? And what do I have to do to validate the user after he logged in then?

    Oh btw, you missed my other question. How do I validate the username so it doesn't get such a query to ruin my database? Is it right to put them between "'s when I query the database? Or can I stop this with that magic quotes thing?

  8. #8
    Keep it simple, stupid! bokehman's Avatar
    Join Date
    Jul 2005
    Posts
    1,935
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Get a proper host!

  9. #9
    SitePoint Member
    Join Date
    Sep 2005
    Location
    Belgium
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by bokehman
    Get a proper host!
    Recommend me one then.

  10. #10
    SitePoint Zealot DewChugr's Avatar
    Join Date
    Sep 2005
    Location
    Illinois
    Posts
    189
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Xargo
    Recommend me one then.
    mediatemple

  11. #11
    Keep it simple, stupid! bokehman's Avatar
    Join Date
    Jul 2005
    Posts
    1,935
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Xargo
    Recommend me one then.
    The truth is if they are good they are usually expensive. Personally I pay for a 24 hour connection and have my own server. I have my own DNS, mail and web server. Every time I add another domain it costs nothing extra.

  12. #12
    SitePoint Member
    Join Date
    Sep 2005
    Location
    Belgium
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I'm sorry, I won't ever pay for a web hosting if there are so many free web hosts out there. I'm already paying for a com domain and in the future I'll have to pay for an mmorpg server too, that's more than enough.

    Btw, lvismer, can you please check my answer?

    Quote Originally Posted by Xargo
    I don't see why you find POST and GET variables set messy. The point is that I use MySQL for the content of my sites, my site is only one page: index.php. I always have to use GET variables. And if I use a login form I certainly need to use POST too, so I can't avoid using both at the same time.

  13. #13
    Keep it simple, stupid! bokehman's Avatar
    Join Date
    Jul 2005
    Posts
    1,935
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Xargo
    The point is that I use MySQL for the content of my sites, my site is only one page: index.php
    Why don't you use the file system? Also you can still have proper URLs without a query string even if you do use a DB.

  14. #14
    so shiny exigent's Avatar
    Join Date
    Nov 2005
    Posts
    296
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Xargo
    I'm sorry, I won't ever pay for a web hosting if there are so many free web hosts out there. I'm already paying for a com domain and in the future I'll have to pay for an mmorpg server too, that's more than enough.

    Btw, lvismer, can you please check my answer?

    The limitations of free hosts are immense. Most kick you out when you use too much bandwidth/space, a lot are scams or they put advertisements on your site.

    Most won't even give you enough "fuel" to run a popular website. If you plan on running a MMORPG, you'll most definately need at least shared web hosting (most likely dedicated if its very popular).


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •