Page 1 of 2 12 Last
  • Jump to page:
    #1
  1. A Change of Season
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Mar 2004
    Location
    Next Door
    Posts
    2,664
    Rep Power
    171

    Security question about simple login script and sessions


    Hi;

    I have no doubts that this can be improved dramatically but for now I have 5 major questions:

    1 - How easy is it for someone to mess with $_SESSION['logged']. By mess I mean 1 - Create it on my server and change its value to TRUE.

    2 - How easy is it for hacker to learn about the name of the session that looks after the login?

    3 - From scale of 0 to 10 how secure is this script?

    4 - Is it true to say there is absolutely no threat of XSS as none of the entries are ever gonna be printed anywhere?

    5 - Does it even matter to have 2 passwords?

    6 - Jaques1, be gentle



    PHP Code:
    <?php session_start();
    include 
    "/home/bmx_bikes/includes/myconnections.php"
    $sql "SELECT password_one, password_two FROM enquieries_password WHERE id = 1";
    $data DB::Load()->Execute($sql)->returnArray();
    if(
    $_POST['submit']=='check')
        {
            if(
    sha1($_POST['password_one'])==$data[0]['password_one'] && sha1($_POST['password_two'])==$data[0]['password_two'])    
                {
                    
    $_SESSION['logged']=TRUE;    
                }
        }
    if(
    $_SESSION['logged']!=TRUE)
        {
            
    ?>
                <form action="<?php echo $_SERVER['PHP_SELF'];?>" method="post">
                    Password 1 <input type="password" name="password_one" /><br />
                    Password 2 <input type="password" name="password_two"  /><br /> 
                    <input type="submit" name="submit" value="check" />
                </form>                   
            <?php    
        
    }
    Thank you
  2. #2
  3. Did you steal it?
    Devshed Supreme Being (6500+ posts)

    Join Date
    Mar 2007
    Location
    Washington, USA
    Posts
    14,066
    Rep Power
    9398
    1. It is not. Assuming a typical configuration where session files are stored on the server, an attacker would have to break into the server itself to get to those files, and if they can do that you're hosed anyways.

    2. Session... id? It's in a cookie. Or what's this "name" you're referring to?

    3. Secure? Two problems. I count overall
    - 2 hashes without salts
    - 2 potential array-to-string conversion warnings (I could manipulate your form so that $_POST["password_*"] were arrays via setting name="password_one[]"). Very minor and nobody, like, ever considers that
    - 1 undefined offset warning, repeated twice (what if there are no matching rows from the query)
    - 1 undefined offset warning (password check fails, $_SESSION['logged'] is undefined)
    - 1 XSS (PHP_SELF)

    4. No: XSS from PHP_SELF.

    5. Not really: you're storing them in the same place, using the same input form, using the same hashing algorithm. You've doubled the length of the password and increased the likelihood that someone will need to write it/them down somewhere to remember. It also looks a lot like you're only confirming the first password so some people may just repeat the one twice.


    Hardcoded id=1?
  4. #3
  5. A Change of Season
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Mar 2004
    Location
    Next Door
    Posts
    2,664
    Rep Power
    171
    Originally Posted by requinix
    2. Session... id? It's in a cookie. Or what's this "name" you're referring to?
    I was thinking if there might be a way for hackers to change the value of $_SESSION['logged'] to TRUE. Looks like it is only possible if they access the server.
    Originally Posted by requinix
    3. Secure?
    - 2 hashes without salts
    - 2 potential array-to-string conversion warnings (I could manipulate your form so that $_POST["password_*"] were arrays via setting name="password_one[]"). Very minor and nobody, like, ever considers that
    - 1 undefined offset warning, repeated twice (what if there are no matching rows from the query)
    - 1 undefined offset warning (password check fails, $_SESSION['logged'] is undefined)
    That was Japanese
    Originally Posted by requinix
    - 1 XSS (PHP_SELF)

    4. No: XSS from PHP_SELF.
    PHP Code:
    function html_escape($raw_input)
        {
            return 
    htmlspecialchars($raw_inputENT_QUOTES ENT_HTML401'UTF-8');     
        } 
    PHP Code:
    echo "action =".html_escape($_SERVER['PHP_SELF']);?>
    Originally Posted by requinix
    It also looks a lot like you're only confirming the first password so some people may just repeat the one twice.
    Why do you say that?
    Originally Posted by requinix
    Hardcoded id=1?
    Yes what's wrong with that?

    Thank you
  6. #4
  7. Did you steal it?
    Devshed Supreme Being (6500+ posts)

    Join Date
    Mar 2007
    Location
    Washington, USA
    Posts
    14,066
    Rep Power
    9398
    Originally Posted by zxcvbnm
    That was Japanese
    - Using sha1() is better than md5() yes, but if you just put the text into it without any kind of modifications then it's still not secure.
    - With the []s in the name PHP will parse the form field as an array. You then pass it to sha1() which expects a string, thus probably triggering a warning about the conversion.
    - If the query doesn't return results then $data will be empty, right? So $data[0] will warn.
    - You set logged=true if the password matches, but you don't set it =false if it does not (at least not in the code you've posted). It's only set in one case but you try to access it regardless.
    Originally Posted by zxcvbnm
    PHP Code:
    function html_escape($raw_input)
        {
            return 
    htmlspecialchars($raw_inputENT_QUOTES ENT_HTML401'UTF-8');     
        } 
    Okay.
    Originally Posted by zxcvbnm
    Why do you say that?
    How many places have you seen where it needs two passwords entered at once? How many of those were using the second one for confirmation of the first and not as a completely different password?
    It's a fact that users are stupid and will do whatever they think they're supposed to do regardless of the instructions, so some of them who see the two fields will think it's a confirmation box.

    Originally Posted by zxcvbnm
    Yes what's wrong with that?
    Nothing's wrong, it's just unusual to see things like that. Out of curiosity, what is this password thing supposed to be?
  8. #5
  9. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    Hi,

    I'm sorry to say, but this isn't secure at all.

    The worst thing is that you've chosen a completely inappropriate way of storing the passwords. No, I'm not talking about salt, pepper or whatever. That wouldn't help you (unless you wanted cook a tasty meal).

    MD5, SHA-1, SHA-2, SHA-3 etc. are no password hashing algorithms.

    I know they've always been abused for this purpose in the PHP world. I mean, md5() has almost become the signature move of PHP "security". And since people have heard that MD5 is somehow "bad", they now seem to move to the SHA family (which somehow they consider to be "good").

    The thing is: None of these algorithms was ever intended or suitable for hashing passwords. They're great for things like secure checksums. So if you wanna let your users check the integrity of some files, SHA is great. But if you wanna store passwords, you're really, really on the wrong track.

    MD5 or SHA are at best password obfuscation algorithms -- they're the equivalent of putting a "Beware of the dog" sign on you fence (or two signs, in your case). That might be a little bit better than doing nothing at all, but it's obviously no replacement for locking your doors.

    Just check this benchmark chart of Hashcat: A standard gamer PC can calculate 2 billion(!) SHA-1 hashes per second. The number of, for example, all 6-character passwords consisting of alphanumerics (lowercase and uppercase) is 62^6≈ 57 billion. In other words, grinding every possible hash of that range only takes about 27 seconds. I bet the boot procedure of you PC takes longer. And that's just the time for actually calculating all hashes ad hoc. An attacker might as well use a precalculated list that will tell them immediately the original password for a given hash.

    As you can see, SHA-1 provides no security whatsoever. Yeah, SHA-2 would increase that time a bit so that "hackers" would now have to wait a minute maybe (damn!). And a salt would force the "hacker" to spend this minute on every password instead of the whole database. But obviously none of this solves the actual problem: You simply got the wrong tool.

    What you want is a password hashing algorithm such as PBKDF2 or bcrypt or scrypt. The good news is: bcrypt is already built into PHP, and there are excellent libraries which make it very easy to use. PHP 5.5 will also have a native password API. The bad news is: The PHP "community" seems to have a very hard time adopting it. Many people still use MD5/SHA or try to come up with some home-made algorithm (which is guaranteed to fail) instead of simple using a proven solution.

    Be smart, use bcrypt. It's fundamentally different from algorithms like SHA:
    • It was actually designed for password hashing.
    • It explicitly supports the use of salts, which means an attacker cannot "crack" all hashes at once or use a precalculated list.
    • It's computationally expensive, which means it will massively slow down brute force attacks. Ideally, it makes them impractical altogether.
    • You can make it even more expensive by increasing the cost factor.
    Last edited by Jacques1; June 21st, 2013 at 05:18 AM.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  10. #6
  11. A Change of Season
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Mar 2004
    Location
    Next Door
    Posts
    2,664
    Rep Power
    171
    Thanks Jaques1. Like always, a great reply. This also is a great resource.

    Jaques,

    I always thought sha1 is not reversable. If it is, would you please also tell us how you go about decrypting this:
    3ade2d3e681143945dcaf2980b87853d0f84fd52

    Also you don't seem to rely purely on this then, do you?
    do_hash()

    Permits you to create SHA1 or MD5 one way hashes suitable for encrypting passwords. Will create SHA1 by default. Examples:
    $str = do_hash($str); // SHA1

    $str = do_hash($str, 'md5'); // MD5
    PHP Code:
    if ( ! function_exists('do_hash'))
    {
        function 
    do_hash($str$type 'sha1')
        {
            if (
    $type == 'sha1')
            {
                return 
    sha1($str);
            }
            else
            {
                return 
    md5($str);
            }
        }

    Thanks
    Last edited by zxcvbnm; June 22nd, 2013 at 02:57 AM.
  12. #7
  13. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    Originally Posted by zxcvbnm
    Thanks Jaques1. Like always, a great reply. This also is a great resource.
    Well, not so sure about that. They say that you should use crypt(), but this is a low-level function not meant for "end users" like us. It's difficult to use and has several pitfalls, which means you can easily end up breaking it.

    What you should actually do is use a wrapper library like the already mentioned password_compat or the older Phpass. Those take care of all the low-level stuff and offer a convenient API.



    Originally Posted by zxcvbnm
    I always thought sha1 is not reversable. If it is, would you please also tell us how you go about decrypting this:
    Your input was "behnam".

    SHA and MD5 are not reversible in the sense that you could actually revert the calculations and up with the original input. But you can simply try out all possible inputs at very high speed until you finally end up with a string that leads to the same hash.

    So in your case, I'd simply go through all alphanumeric strings "0", "1", ..., "a", ..., "z", "A", ..., "Z", "00", "01", ..., "ZZ", ..., "000", "001"..., "ZZZ", ... and calculate their SHA-1 hashes. You can do that with tools like Hashcat or John the Ripper. As soon as the hash is "3ade2d3e681143945dcaf2980b87853d0f84fd52", I know that you (probably) entered this input. At least I have a string that your password check would accept.

    Note that SHA-1 hashes are not unique, because they map a very large range of possible inputs to only 160 bits. So there are necessarily collisions (different inputs with the same hash).



    Originally Posted by zxcvbnm
    Also you don't seem to rely purely on this then, do you?
    That "helper" is laughable. Plain hashes with no salt may have been acceptable in the 90s, but certainly not in the 21. century.

    Security-wise, CodeIgniter is pretty crappy.
    Last edited by Jacques1; June 22nd, 2013 at 05:14 AM.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  14. #8
  15. No Profile Picture
    Lost in code
    Devshed Supreme Being (6500+ posts)

    Join Date
    Dec 2004
    Posts
    8,317
    Rep Power
    7170
    For nearly all short unsalted md5 or sha1 hashes, all you have to do is Google the hash to determine the source.
    PHP FAQ

    Originally Posted by Spad
    Ah USB, the only rectangular connector where you have to make 3 attempts before you get it the right way around
  16. #9
  17. A Change of Season
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Mar 2004
    Location
    Next Door
    Posts
    2,664
    Rep Power
    171
    Jaques1;
    Beautifully explained. Do you consider this as a good resource?

    E-Oreo:
    Your always right too bud.

    requinix:
    I dont remember you ever speaking about SHA1 the way Jaques did. Did you know these? I vaguely recall you supporting using only salted SHA1. I might be wrong. I am probably wrong. yes I'm wrong no way you didn't know this; did you?

    Bonus
  18. #10
  19. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2011
    Location
    United Kingdom
    Posts
    42
    Rep Power
    5
    PHP Code:
    DB::Load()->Execute($sql)->returnArray() 
    You shouldn't chain-load like this buddy. If Load() or Execute() return null or a non-object then you'll get a fatal error call to member function on non-object. FYI.

    Comments on this post

    • Northie disagrees : I wrote the code zxcvbnm uses here. It's rock solid. FYI chaining objects is one of the best things about OOP and was 'designed in'. Look at JavaScript for how an OO language should work
    • Jacques1 disagrees
  20. #11
  21. No Profile Picture
    Lost in code
    Devshed Supreme Being (6500+ posts)

    Join Date
    Dec 2004
    Posts
    8,317
    Rep Power
    7170
    I dont remember you ever speaking about SHA1 the way Jaques did. Did you know these? I vaguely recall you supporting using only salted SHA1. I might be wrong. I am probably wrong. yes I'm wrong no way you didn't know this; did you?
    SHA1 is still used extensively for password hashing, both in the PHP world and outside of it, because it's extremely portable. Not everyone agrees with Jacques1's opinion that it is not suitable for password hashing; if that opinion were universal, then libraries like phpass and PHP's crypt() function would not provide SHA1 as an option (they do). However, the point about SHA1 being quick to test is universally recognized as a problem, which is why these libraries apply a cost factor to it by repeating the SHA1 hash hundreds of thousands of times.

    Comments on this post

    • Jacques1 disagrees : I think you're again doing more harm than good... :-(
    PHP FAQ

    Originally Posted by Spad
    Ah USB, the only rectangular connector where you have to make 3 attempts before you get it the right way around
  22. #12
  23. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    OK, where do I start ...

    Originally Posted by E-Oreo
    SHA1 is still used extensively for password hashing, both in the PHP world and outside of it, because it's extremely portable.
    What? You seem to be confusing SHA-1 with SHA-2.

    SHA-1 is exotic, you'll have a hard time finding it in the wild. And that's a good thing, because it has several known weaknesses, some of which can speed up brute force attacks by 20%.

    Typical weak algorithms used today are MD5 and SHA-2. And they're mainly used because people simply don't know any better.



    Originally Posted by E-Oreo
    Not everyone agrees with Jacques1's opinion that it is not suitable for password hashing; if that opinion were universal, then libraries like phpass and PHP's crypt() function would not provide SHA1 as an option (they do).
    Neither PHPass nor crypt() "provide SHA-1 as an option". No idea where you got this from.

    What PHPass and crypt() do have is custom algorithms using MD5 or SHA-2 as building blocks -- just like bcrypt is based on the Blowfish encryption algorithm. Using this to claim that MD5 or SHA-2 are good hash algorithms is like saying that bit shifting is a good encryption scheme because it's used in AES. It's a least ... misleading.

    MD5 and SHA are not and will never be acceptable password hashing algorithms. We've just seen that with zxcvbnm's test string. Cryptographers can use them as primitives to build good password hashing algorithms -- just like engineers use screws to build airplanes. That doesn't make screws a tool for flying. In pratical terms: If you're having a sha*() or md5() somewhere in your password checking code, you're doing it wrong.

    I have no idea why you're having such a hard time accepting bcrypt and why you stab me in the back every time I suggest security improvements. What's you problem with bcrypt?

    And, please, stop saying that this is just my "opinion". bcrypt is probably the most widely accepted password hashing algorithm (together with PBKDF2). When you call that an "opinion", it reminds me of the crazy creationist people you have in the USA, who'll tell you that evolution is "just a theory".



    ------



    Originally Posted by zxcvbnm
    Jaques1;
    Beautifully explained. Do you consider this as a good resource?
    Yes!
    Last edited by Jacques1; June 23rd, 2013 at 01:22 PM.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  24. #13
  25. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    On a side note:

    Originally Posted by deav
    You shouldn't chain-load like this buddy. If Load() or Execute() return null or a non-object then you'll get a fatal error call to member function on non-object. FYI.
    Northie already commented on this, but you clearly misunderstand OOP.

    In object-oriented programming, you signal an error by throwing an exception. You do not return false. This practice is only used in primitive procedural languages which have no real error system and instead need to rely on return values.

    Unfortunately, many PHP libraries -- including the built-in ones -- are a weird mixture of modern OOP and classical procedural programming. Just like PHP itself. So PHP people have gotten used to getting strange errors about "non-objects" because of some method still using the procedural approach. But that's wrong. In properly written OOP code, you should never run into a method that returns false instead of an object.

    In reality, this won't always work out, because PHP simply isn't an object-oriented language. It's a procedural language with some objects bolted on, so it will probably always suffer from the paradigm clash. But at least custom methods like these should be written correctly and throw an exception in case of an error. Then you can safely chain methods, because if any of them fails, the whole chain will be cancelled.

    And method chaining is indeed one of the best features of OOP as it allows for a nicely "flowing" programming style.

    Comments on this post

    • Northie agrees : Thanks for the vote of confindence
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  26. #14
  27. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2011
    Location
    United Kingdom
    Posts
    42
    Rep Power
    5
    Originally Posted by Jacques1
    On a side note:

    Northie already commented on this, but you clearly misunderstand OOP.
    So you're telling me that a function like loadArray which looks to return data (or no data) from a database should throw an exception if no records are found? Are you mad?

    How would this work, would you just throw an exception if you can't find any records?

    Returning false shouldn't be used in OOP? **** me have you ever written a line of production ready code?

    If I want to check to see if a variable is set on an object using a getter, would I throw an exception if it's set to false? Or return false?
  28. #15
  29. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    I have no idea what you're reading into my post ...

    OK, let's go back to your first post: You told zxcvbnm to not use method chaining, because one of the two "inner methods" might return null or a "non-object" like false. I'm telling you that this cannot happen in properly written OOP code.

    DB::Load() and $db->Execute() are supposed to return a database object and a result set object respectively. The only situation when you'd not do that and instead return something like null is because there was an error: The database connection or the query failed.

    So the only way you'd get the "non-object" message you're talking about is if the author of the methods used null and false for signalling errors. And this would be wrong. In OOP, you signal errors with exceptions. If you can't connect to the database or execute the query, you throw an exception. You do not return null or false.

    In other words, when dealing with proper OOP code, you never run into this "non-object" error when chaining methods that are supposed to return objects. Every method either returns an object or throws an exception. It doesn't return null or something. So you can safely chain them.

    Of course some methods are supposed to return a boolean value or null. Those obviously cannot be chained. But since zxcvbnm doesn't do that, the code above is absolutely correct.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
Page 1 of 2 12 Last
  • Jump to page:

IMN logo majestic logo threadwatch logo seochat tools logo