When a user logs in, I save a token (hashed password) to the user's folder on the server and send the token also to the app which is saved in localStorage.
I need to check if the user's login token is valid upon every request made to the server. It's requested via AJAX using setTimeout, so next request is made only if previous call is completed (every 15 seconds).
Currently I have this function that verifies the token i.e checks if the token from the app equals the one on the server:
<?php if( verifyToken()) { //Success
} else {
//die
}?>
Below is the function verifyToken
function verifyToken() {
if ( empty( $_REQUEST['token'] )
|| empty($_REQUEST['username'] ) )
{
return false;
}
$username = $_REQUEST['username'];
$user_token = $_REQUEST['token'];
$secure_file = getUserDir( $username) . "/secure__.php";
if ( !file_exists( $secure_file) )
{
return false;
}
$token = file_get_contents( $secure_file);
if ( !$token )
{
return false;
}
if( $token==$user_token )
{
return true;
}
else
{
return false;
}
}
Does my approach above have disadvantages or should I consider checking against the database every 15 seconds?
I don't want to use sessions.
1 Answer 1
The token is a "password" that is sent and saved in plain text
Let me explain what I mean. Let's say a hacker gets access to your server, similar to what happened to GoDaddy (as an example off the top of my head). You're in a terrible position. The hacker doesn't have to do any cracking and can simply write a call like this to log in as any user using the exact values they got from the files:
curl -d username=the_username -d token=the_token your.example.com
It is vulnerable to timing attacks
The whole plain text thing mentioned above means that it's pretty easy to do a timing attack, made possible by the fact that you are using ==
(or ===
) to check it. Any attacker can start by guessing different first letters and as soon as the response is even the slightest bit slower, they will know they have the right one. Rinse and repeat for the rest. They probably already know how long the token is supposed to be given that they can inspect what the AJAX call is sending.
Crackers will run with this
The ability to log into your system probably isn't as useful as the original password. Fortunately for the attacker, they have the hash of that and can get to cracking offline. Then they can try it on other sites to see who's reusing passwords.
You should be following password best practices
You can use password_verify to compare the password you got from the request to the stored hash of said password (note the mention that the function protects from timing attacks). Alternatively, use a solution that will do these things for you, one vetted by the community for security (e.g. an open-source framework).
Other Changes
You will have to do major rewrites to fix the major problems listed above. Here are the suggestions I have based on the rest of the code:
- Indent consistently. Your code is very, very hard to read because of your formatting. You can use an online prettifier to see what good formatting looks like.
- Use return types (
function verifyToken(): boolean {
) - Use triple equals instead of double (but see above for the note about timing attacks)
-
1\$\begingroup\$ I agree with everything you said, but I don't see any usage of plain passwords? Are you referring to the "token" which is a hashed password? I agree it's a bad idea to store a hash of the password on the user's computer, but a hash is not plain text. Remember this is code to only verify that an user is already logged in, we don't know what the code for logging in looks like. \$\endgroup\$KIKO Software– KIKO Software2022年04月15日 06:41:39 +00:00Commented Apr 15, 2022 at 6:41
-
\$\begingroup\$ @KIKOSoftware OP's comment says that the token is the only thing that's needed to get meaningful data. That means it is a password (albeit one not reused on other sites) and the attacker would have full access to the system with it. And anyone who wants the original password is going to be cracking their hashes offline. \$\endgroup\$Laurel– Laurel2022年04月15日 11:14:12 +00:00Commented Apr 15, 2022 at 11:14
-
\$\begingroup\$ Yes you're right, the token does behave like a password. \$\endgroup\$KIKO Software– KIKO Software2022年04月15日 11:21:12 +00:00Commented Apr 15, 2022 at 11:21
-
\$\begingroup\$ @Laurel Great details...Do you however mean i should store the user's plain password( e.g. 12345) in localStorage when login, and send it along whenever request is made to the server to password_verify it? Though it's an Android app and i also read one shouldn't store plain password in localStorage. Is the user safe with that? \$\endgroup\$The concise– The concise2022年04月16日 07:36:14 +00:00Commented Apr 16, 2022 at 7:36
-
\$\begingroup\$ @Laurel Also note that the token is generated when loging in and it's temporary i.e it's regenerated upon each login and saved in a file on server. It's actually different from the hash stored in Database upon registration. The temporary one is used just for data requests by app. Even the temporary token is a hashed randomly generated characters (uniqid() ) and not the direct password used for login in. \$\endgroup\$The concise– The concise2022年04月16日 08:04:20 +00:00Commented Apr 16, 2022 at 8:04
$_REQUEST
? Are you not in complete control regarding how this payload of data is being sent to your server? Have you read PSR-12 coding standards? \$\endgroup\$