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;
2 Answers 2
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.
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
-
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– vp_arth2015年11月07日 19:06:12 +00:00Commented Nov 7, 2015 at 19:06 -
\$\begingroup\$ v@vp_arth Ok, you are right, didn't knew that. \$\endgroup\$Brain Foo Long– Brain Foo Long2015年11月07日 21:44:46 +00:00Commented 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\$Nobody moving away from SE– Nobody moving away from SE2015年11月08日 10:38:37 +00:00Commented Nov 8, 2015 at 10:38