7
\$\begingroup\$

I'm learning PHP now, and I'm trying to find out whether or not the following code structure has any faults:

$loggedIn = false;
if (isset($_GET["userpass"])){
    if (strcmp($_GET["userpass"], $correctPassword) == 0) {
        $loggedIn = true;
    }
}

To me, it seems pretty bullet-proof. If the password isn't set, then the code doesn't execute. And if it is, then the user actually has to supply the correct password. Anything obvious I'm missing?

Note: $correctPassword is defined in an external file, not accesible to the public

Update: Can the user of this website explit the script so that he can log in to any arbitrary account? As in, this piece of code always sets $loggedIn = true;

\$\endgroup\$

2 Answers 2

15
+200
\$\begingroup\$

This is not secure at all.

Apart from the GET issue (use POST) and the hashing issue (use bcrypt) already mentioned, and the fact that == is not timing safe (use hash_equals), an attacker can log in without knowing the password like this:

login.php?userpass[]=1

The reason for this is that strcmp("STRING", [ARRAY]) returns NULL, and NULL == 0 is true. You should pretty much never use ==. Always use === for type-safe checks.

\$\endgroup\$
0
9
\$\begingroup\$

Checking passwords / sending passwords via $_GET (in URL) as a absolutely nogo. You have to send that credentials via $_POST, using a <form> or Ajax. You also don't need to use strcmp(), a simple comparison for example if("pw" === "pw") is enough.

Why is $_GET or passwords in url bad? So many reasons, it is just evil. Example, every logging between you and the website (browser, your provider, proxy, vpn, etc..) can see the password.

Another thing as that you never ever should save a password in plain text wherever it is possible. Always use a hashing algorithm with salts. Example: http://php.net/manual/en/function.password-hash.php

\$\endgroup\$
3
  • 4
    \$\begingroup\$ You're wrong about also when your site use https. True reasons of $_GET usage is bad are 1. server logs, 2. browser cache, 3. leakage referers. \$\endgroup\$
    – vp_arth
    Commented Nov 7, 2015 at 19:06
  • \$\begingroup\$ v@vp_arth Ok, you are right, didn't knew that. \$\endgroup\$ Commented Nov 7, 2015 at 21:44
  • 1
    \$\begingroup\$ To add to @vp_arth's comment: Without https, using POST instead of GET still leaves the data visible (see this SO answer) to all parties that are involved in the transportation of it. The only advantages (IMHO) of POST over GET are less restrictions on length/format and avoiding accidental publishing of your credentials when copying from the address bar to publish a link. \$\endgroup\$ Commented Nov 8, 2015 at 10:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.