Login process always works, even with wrong password/username...

Hello :slight_smile:

I’m trying to make a registration/login scene for my game. Unfortunately, the player always will
forwarded even if the username/password or even
both are wrong.

To implement this I followed a tutorial, but it is
not saying whether the same problem exists there or not.

The whole thing is made up of two problems.
Problem 1 (see image) is that the ‘hash’ field in the database has ‘*0’
is entered, but in the tutorial it looked different (similar to the entry in the ‘salt’ field).

In the end, the player is redirected, although
incorrect data may have been entered.
Can someone take a look on my code to tell me where the errors are?

Kind regards
Toby

This could should name the username after login was successful.

    public Text playerDisplay;

    public void Start()
    {
        if (DBManager.LoggedIn)
        {
            playerDisplay.text = "Welcome Commander " + DBManager.username;
        }
    }

This code contains the LogIn Function/Coroutine

 // Login
public void CallLogin()
{
     StartCoroutine(LogIn());
}

IEnumerator LogIn()
{

     WWWForm loginForm = new WWWForm();
     loginForm.AddField("name", username_field.text);
     loginForm.AddField("password", password_field.text);

     UnityWebRequest wwwlogin = UnityWebRequest.Post("http://localhost/sqlconnect/login.php", loginForm);
     yield return wwwlogin.SendWebRequest();

     if (wwwlogin.result == UnityWebRequest.Result.Success)
     {
         DBManager.username = username_field.text;
         // DBManager.score = int.Parse(wwwlogin.downloadHandler.text.Split('\t')[1]);
         UnityEngine.SceneManagement.SceneManager.LoadScene(1);
     }
     else
     {
         Debug.Log("User LogIn failed... " + wwwlogin.error);
     }

}

This is the database manager file.

public static class DBManager
{

    public static string username;
    public static int score;

    public static bool LoggedIn {  get {  return username != null; } }

    public static void LogOut()
    {
        username = null;
    }

}

This is the php file for the login process

<?php

$con = mysqli_connect('localhost', 'root', 'root', 'unityaccess');

//check connection
if (mysqli_connect_errno())
{
echo "Error #1"; // error code #1 = Connection failed!
exit();
}

$username = $_POST["name"];
$password = $_POST["password"];

// check if name exists
$namecheckquery = "SELECT username, salt, hash, score FROM players WHERE username='" . $username . "';";

$namecheck = mysqli_query($con, $namecheckquery) or die("Error #2a"); // error code #2a = name check query failed

if (mysqli_num_rows($namecheck) <1)
{ 
echo "Error #3a"; // error code #3a = name does not exist or something (un)funny is going on (>1)...
exit();
}

if (mysqli_num_rows($namecheck) >1)
{ 
echo "Error #3b"; // error code #3b = something (un)funny is going on... (too many results?!?)
exit();
}

// get login info from query
$existingInfo = mysqli_fetch_assoc($namecheck);
$salt = $existingInfo["salt"];
$hash = $existingInfo)["hash"];

$loginhash = crypt($password, $salt);
if ($hash != $loginhash) {
echo "Error #6"; // errorcode #6 = incorrect password
exit();
}

echo "0\t" . $existingInfo["score"];

?>


9768552–1399608–DBManager.cs (340 Bytes)
9768552–1399611–Register_LogIn_Menu.cs (2.7 KB)
9768552–1399614–MainMenuScript.cs (773 Bytes)

You check

if (wwwlogin.result == UnityWebRequest.Result.Success)

you are not checking the content of the response. ergo it could say any of your response texts and still be treated as good - hence your issue. You may also want to keep it simpler, instead of trying to parse some long complex string to work out what failed, just have error codes.

such as 0 = success, 3a/b/6 should all report the same do not let people fish for IDs… so pick a number 22 or something for failed match, and 99 for all hells broken loose and nothing makes sense any more… just return the int. for all the non successes, return -1 as the score.

so you get your webcall, if success the web page didnt totally barf, parse the response, and split with a tab (as you’ve picked tab) and if [0] == 0 then good, else bad luck try again

Yourself? :hushed:

a) this is far from easy because you also need to adhere to security standards (prevent code injection, ie users might register with usernames/passwords that compromise your system) and comply to privacy regulations (ie DSA requirements, enabling players to request what you store about them, to delete their account)
b) there are existing solutions / integrations
c) players expect to be able to use their existing accounts ie Google, Apple, Steam, you name it

On first sight I see you are accepting username and password from the user as is. This is an obvious security flaw! Imagine with little knowledge of SQL a user might insert something like this (may or may not be valid SQL but with minimal alteration it will be possible):

$namecheckquery = "SELECT username, salt, hash, score FROM players WHERE username='" . $username . "';";

where provided username might be:
* from players; DROP TABLE players; SELECT hahaha

Your server PHP is now going to execute a three-fold SQL statement:
$namecheckquery = "SELECT * from players; DROP TABLE players; SELECT hahaha, salt, hash, score FROM players WHERE username='" . $username . "';";

Guess what? This will delete the players table contents and ANY user can do that! :face_with_spiral_eyes:

This attack is called SQL injection and I can guarantee you that within the first few hundred players to sign up, there’s going to be one who will try that.

Perhaps you did some cleanup of the username elsewhere but if this is done user-side, it can be disabled by the user too.

2 Likes

On the topic of cleanup it’s common in my experience to see people talking about “preventing” SQL injections with basic cleanup like escaping user input strings but the best approaches are prepared statements (parameterized queries) and stored procedures. Anything less and you’re just wasting your time as your system will be attacked.

https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html
https://www.php.net/manual/en/pdo.prepared-statements.php

1 Like

Firstly, thank you for your replies :slight_smile:

I took a closer look at the link in the third post.

Of course you are right when you mention the obvious security risks.

But for now I want to finish the tutorial because as a total beginner I need a reference, even if it means having a reference on how not to do it.

I’ve only just reached the middle of the video series and I think there’s another one coming that’s dedicated to safety.
Unfortunately, I can’t say for sure right now because I’m not at home.

So first I would like to get the registration/login up and running, complete the tutorial and understand every aspect of the code. Then I also take care of the security gaps.

So I first have to see how I can change the code so that it runs.
The author’s code seems to work and my database entries are the same, so I don’t understand why the hash entry in the database looks different than his.
I’ll do that as soon as I get home.

Thank you also for raising the safety concerns. I’ll definitely take care of it as soon as the login itself works and I understand everything :wink:

2 Likes

I just saw that the last video is called ‘Avoid SQL Injection’. So I’ll postpone the security issue until this point and see that everything else works, otherwise I’ll get confused. Thanks again for the answers, I’m now taking the security issue even more seriously than before.

Currently all your webstuff is local, so, only you can hack it… thats fine… hopefully you understood what i said as to why it wasnt working from a moving forward and getting it to work perspective

For some reason I can’t edit the post so I have to make a new one again.

The problem is solved, I simply missed the exit(); in login.php.

This caused the game to display the error, but let the script continue to run anyway.
It’s embarrassing, but now everything works, both correct and incorrect input.
Of course, the security question remains for now, but that would go beyond this topic, especially
since I’m not ready yet and want to finish the tutorial, which has a video about this.

Thanks again, I hope one day I’ll be good enough to help others here too :slight_smile:

2 Likes

Unfortunately I have to back out again because it doesn’t work. Another mistake by myself caused this (what else^^).

It looked like it was working, but everything was blocked regardless of whether the password was correct or not.
The strange thing is that it doesn’t matter if I enter a wrong nickname or a wrong password, suddenly it always goes through, which means that the exit() never works. But it worked before, when the password check failed?!?

If I understood correctly, the login.php checks
how many matches there are to the nickname entered
from the text field. If this value is < or > 1 (!= would of course also work), the login attempt should already be aborted, especially because the nickname is not available (greater than 1 should not work).
However, this does not happen. But when I load the login.php itself, it shows me error code #3a, since the nickname is inevitably empty. I don’t know why this doesn’t work in the game.
The same applies to registration. It’s a little tricky though
it always shows that it was successful.
The only reason why there is no new entry in the database is that the entries are set to unique.

But since these functions worked before and I didn’t change anything, I’m stuck (again).

@bugfinders : Unfortunately I didn’t see your post in time, otherwise I would have referred to it directly. However, I’m not 100% sure what exactly you mean, I understood it to mean that I’m completely missing a query about the content. But that’s exactly what I did, namely checked whether the text I entered matched database entries.
So I’m afraid I didn’t fully understand you, sorry :frowning:

Usually I would say it is the better (or the only) way to make it correct rightaway, but
as I want to follow the tutorial I am sure I cant follow it if I change too much…

Either way, it’s not clear to me where the problem exactly is, since there is a query for matches.
Herewith:

{ 
echo "Error #3a"; // error code #3a = name does not exist
exit();
}```
(Example from Login.php)

Last word: Is there a way to change the topic prefix to 'blind author' or something similar? :smile:

well…

as I said, you only check that the webpage returns status 200… you dont check what it has to say inside.

eg here

 if (wwwlogin.result == UnityWebRequest.Result.Success)
     {
         DBManager.username = username_field.text;
         // DBManager.score = int.Parse(wwwlogin.downloadHandler.text.Split('\t')[1]);
         UnityEngine.SceneManagement.SceneManager.LoadScene(1);

your response code from the website could say “F off you hacker” and it would still set the user name to the crap typed in and load the next scene

1 Like

Ah okay. Then it was explained incorrectly in the tutorial.
If I had more experience I might have figured it out myself. This means that the C# code allows the user to pass through, even if an exit() is applied in Register.php or Login.php, even though that was actually intended
an error message is output with exit() and the process is stopped? By the way, thank you for your patience with me :wink:

Since I’m mainly a php developer at the time and I actually implement user authentication in our system, let me mention a few things here. A lot of points have already been made but I want to summarize and add a few more.

  • SQL querys in php are usually best done with PHP’s “PDO” classes (PHP Data Objects). They are an abstraction and work with several different SQL servers.
  • When using PDO, you usually use “prepared statements” where you actually insert place holders in your query and use one of the bind methods to actually safely provide the value for those place holders, or pass the values to the execute method which will bind them right before the execute.
  • When you design a web API, you should follow certain conventions. Currently you just “echo” some content and terminate the execution after that. The HTTP protocol has distinct return codes (1xx - info, 2xx - success, 3xx - redirect, 4xxx client error, 5xx - server error). Note that “server error” does only apply to an actual issue / problem / bug on the server which is the servers fault. When the user / client has done something wrong, it’s always some 4xx code. Everybody probably knows 404 which means the requested resource does not exist. However 400 is just a “generic” user error while 200 is just a generic success. See HTTP response codes for more details. You set the response code through the http_response_code method in PHP. By using an actual error code, your check in Unity would probably result in a ProtocolError instead of Success.

In order to safely utilize your database, you should create a pdo object like this:

$host = "localhost";
$dbname = "unityaccess";
$dbuser = "root"; // please change that
$dbpassword = "root"; // certainly change that ^^
$pdo = new PDO( "mysql:host=$host;dbname=$database;charset=utf8", $dbuser, $dbpassword);

With that object you can simply do

$stmt = $pdo->prepare("SELECT username, salt, hash, score FROM players WHERE username=:username");
if (!$stmt->execute([":username" =>$username])){
    // error during sql request.
    // only happens when something is wrong, like missing table or column or malformed query
    http_response_code(500);
    exit("SQL request failed");
}
// gets the first result. There's also fetchAll when multiple rows may be returned
$result = $stmt->fetch();
if (!$result){
    http_response_code(400);
    exit("wrong login");
    // if you want to reveal information about what users may exist, you can also return the following
    //exit("user doesn't exist");
}
// do your PW check
if (NotCorrectPW){
    http_response_code(400);
    exit("wrong login");
    // if you want to reveal information about what users may exist, you can also return the following
    //exit("wrong password");
}
// hey, login successful, return the success content after the following line
http_response_code(200);

Note that nowadays it’s common to use json to transmit data as it’s much easier and more robust to parse it on the Unity side. PHP can help you with most of these things. For example

echo json_encode([
    "success" => true,
    "user" => $username,
    "score" => $yourScore,
]);

This would return a json object like this:

{"success":true, "user":"Steve", "score":"123"}

Though the design of your API is up to you.

2 Likes

Thanks for the contribution.
I haven’t dealt with JSON at all yet, but I will certainly do that.

Even though it will take time, I want a solution that works and I want to have a method that doesn’t cause security risks, so I’ll take the time.

However, I’m surprised that none of these points were mentioned in the tutorial. My code is different from the author’s
but only a little, mostly in that I when you register you also want to provide an email and check it.
I also combined everything, i.e. registration and login, in one scene. But that alone shouldn’t lead to such a difference. However, it works for the author.
I have to say that doesn’t make sense to me.
Could it be because he uses WWW instead of UnityWebRequest, since the former is now outdated?

Anyway, I’ll be happy to try your solution.
I’ll do that tomorrow in peace (it’s 9 o’clock in the evening here).
Just to be sure, one more question:
My php query doesn’t work because still
the http_response_code 200 is returned no matter what
happened in the .php file? That would mean that if
I would write http_response_code(200); instead of or in addition to exit(), the program should work?
Or is there also an error in my .php. code that I’m too stupid to see?

I would still prefer your solution because, according to what you said, it seems more modern and safer to me, but if I don’t ask, I might leave another question unanswered, which would hardly get me anywhere.

Thank you very much for the detailed post Bunny83 :slight_smile: :slight_smile: :slight_smile:

OK so error 200 means no error… it you have a text file that says hello, thats the response code because its all good. actually messing with http response codes isnt the way to go, you need to process the content… and whether you use json, xml, text, whatever, you just need to pick and process it…

as for the time… you’re only an hour ahead of me… Im likely up for another 4-5 hours yet :smile:

Other than the hour difference, it’s pretty much the same here. Guess today is one of those days where you don’t stop because you’re making progress…

Hmm, isn’t the response code ultimately the difference between Bunnys and my example?

if (NotCorrectPW){
http_response_code(400);
exit("wrong login");```

I don't know exactly where the ```NotCorrectPW``` comes from, but in the end it's just an ```exit()``` and an ```http_response_code(400)```,
after the password check failed?

So what would be the difference between:
```if (mysqli_num_rows($namecheck) <1)
{
echo "Error #3a"; // error code #3a = name does not exist
http_response_code(400);
exit();
}```

and the following?:
```// do your PW check
if (NotCorrectPW){
http_response_code(400);
exit("wrong login");
// if you want to reveal information about what users may exist, you can also return the following
//exit("wrong password");```

In the end, the difference has to lie in the method behind ```NotCorrectPW```. But this difference is not clear from the example. So what should I use instead of ```if (mysqli_num_rows($namecheck) <1)```?Maybe I suddenly went blind, I heard that's supposed to happen temporarily with developers :smile:

I only see the code response difference and how Bunny is coding, not the difference in how he wants to check the pw?

well. you didnt have any response code stuff in the php you pasted - so… hence it all would return 200 and pretend all was right in the world. the new code and your proposed would result in roughly the same in both forms but like I said you should never tell users they got their username wrong specifically, or their password specifically, only that the combination is not accepted… (again security) …

but in essence if you get a row… its then down to you to check things such as maybe its a shared login so does it have this game? maybe its disabled? maybe the password doesnt match anyway, after all the checks, throwing an error 400 is a reasonable thing to do - you then pick up there was an error in the unity side and say “no go”

1 Like

I have now adapted the code accordingly.
A corresponding text was already displayed in the game, but
I also changed the echo() output in the .php files accordingly. With the change, a large part of the query also works
regarding nickname and email.

However, I still have a problem with the password because no matter what I enter, the program lets me through.

I can only imagine that this is due to the 'hash', which was stored in the database with *0, which was different for the tutorial author, where the entry was more similar to what was entered for 'salt', for example: ‘$5ounds=3750$steamedhamsPlaytester$’.
Does anyone have an idea?

As soon as this section works I will create a new Register.php and
Login.php and try to implement Bunny83’s suggestions.

Here is the new code:
Register.php:


$con = mysqli_connect('localhost', 'root', 'root', 'unityaccess');

//check connection
if (mysqli_connect_errno())
{
echo "Error #1"; // error code #1 = Connection failed!
http_response_code(400);
exit();
}

$username = $_POST["name"];
$email = $_POST["email"];
$password = $_POST["password"];

// check if name exists
$namecheckquery = "SELECT username FROM players WHERE username='" . $username . "';";

$namecheck = mysqli_query($con, $namecheckquery) or die("Error #2"); // error code #2a = name check query failed

// check if email exists
$emailcheckquery = "SELECT email FROM players WHERE email='" . $email . "';";

$emailcheck = mysqli_query($con, $emailcheckquery) or die("Error #2"); // error code #2b = email check query failed

if (mysqli_num_rows($namecheck) > 0)
{ 
echo "Error #3, please try again."; // error code #3a = name already exists 
http_response_code(400);
exit();
}

if (mysqli_num_rows($emailcheck) > 0)
{ 
echo "Error #3, please try again. "; // error code #3b = email already exists 
http_response_code(400);
exit();
}

// add user to the database/table
$salt = "\$5\rounds=3750\$" . "steamedhams" . $username . "\$";
$hash = crypt($password, $salt);
$insertuserquery = "INSERT INTO players (username, hash, salt, email) VALUES ('" . $username . "', '" . $hash . "', '" . $salt . "', '" . $email . "');";
mysqli_query($con, $insertuserquery) or die("Error #4"); // error code #4 = insert player query failed

echo ("User created successful :)");

?>```

Login.php:
<?php $con = mysqli_connect('localhost', 'root', 'root', 'unityaccess'); //check connection if (mysqli_connect_errno()) { echo "Error #1"; // error code #1 = Connection failed! http_response_code(400); exit(); } $username = $_POST["name"]; $password = $_POST["password"]; // check if name exists $namecheckquery = "SELECT username, salt, hash, score FROM players WHERE username='" . $username . "';"; $namecheck = mysqli_query($con, $namecheckquery) or die("Error #2"); // error code #2a = name check query failed if (mysqli_num_rows($namecheck) <1) { echo "Error #3, nickname or password is incorrect"; // nickname does not exist http_response_code(400); exit(); } if (mysqli_num_rows($namecheck) >1) { echo "Error #3, nickname or password is incorrect"; // too many names http_response_code(400); exit(); } // get login info from query $existingInfo = mysqli_fetch_assoc($namecheck); $salt = trim($existingInfo["salt"]); $hash = trim($existingInfo["hash"]); //$salt = $existingInfo["salt"]; // old //$hash = $existingInfo["hash"]; // old $loginhash = crypt($password, $salt); if ($hash != $loginhash) { echo "Error #3, nickname or password is incorrect"; // incorrect password http_response_code(400); exit(); } echo "0\t" . $existingInfo["score"]; ?>

Register_LogIn_Menu.cs:

```csharp
 // Registration
public void CallRegister()
{
     StartCoroutine(Register());
}

IEnumerator Register()
{

     WWWForm form = new WWWForm();
     form.AddField("name", username_field.text);
     form.AddField("email", email_field.text);
     form.AddField("password", password_field.text);

     UnityWebRequest www = UnityWebRequest.Post("http://localhost/sqlconnect/register.php", form);
     yield return www.SendWebRequest();

     if (www.result == UnityWebRequest.Result.Success)
     {
         Debug.Log("Registration complete...");
     }
     else
     {
         processDisplay.text = "Error, please try again with different entries or login...";
         Debug.Log("Registration failed..." + www.error);
     }

}

// Login
public void CallLogin()
{
     StartCoroutine(LogIn());
}

IEnumerator LogIn()
{

     WWWForm loginForm = new WWWForm();
     loginForm.AddField("name", username_field.text);
     loginForm.AddField("password", password_field.text);

     UnityWebRequest wwwlogin = UnityWebRequest.Post("http://localhost/sqlconnect/login.php", loginForm);
     yield return wwwlogin.SendWebRequest();

     if (wwwlogin.result == UnityWebRequest.Result.Success)
     {
         DBManager.username = username_field.text;
         // DBManager.score = int.Parse(wwwlogin.downloadHandler.text.Split('\t')[1]);
         UnityEngine.SceneManagement.SceneManager.LoadScene(1);
     }
     else
     {
         processDisplay.text = "Nickname or password incorrect, please try again...";
         Debug.Log("Login failed, nickname or password incorrect... " + wwwlogin.error);
     }

}

Edit: How did you choose php as a code? I only have 3 options and none of it is php…

Well, I simply used

[code=PHP]
// code

[/code]
Though this site doesn’t have syntax highlighting for PHP so it’s just a code section

About your actual issue: I never used the crypt method, especially since the behaviour seems to be quite vague. Anyways, the crypt method is not meant for a separately stored salt. The algorithm and salt is stored in the hashed string and you should use password_verify to check a plain text password against a hashed password. It will use the algorithm and hash from the hashed string which consists of several parts.

What tutorial do you follow? It seems to be either very outdated or the tutor doesn’t really understand what he’s doing, so probably not a good source.

At my work we used our own algorithm based on sha hashes. Originally we implemented the salt generation, password hashing and checking as stored procedures / function directly in the MySQL database. Though I recently implemented the same in PHP and the old salts and hashes still work. I don’t want to reveal any details here, but as it was mentioned already, hashing passwords is a kinda tricky thing and difficult to get right. Especially when you don’t understand the implications and how “strong” a certain method is.

When you say the login is always successful, does that mean you actually get the content you echo at the end? So a 0 followed by a tab followed by the score of the player? Currently you have commented out the “parsing” of the text your receive. In any way you have to debug what actually happens. Note that you can use error_log("some text");
in php to write to the server’s error log file. This can also help to identify issues on the server side. Also use more Debug.Logs on the Unity side, print out the content you got, etc. We can’t really do any of that for you :slight_smile:

I’m currently using this tutorial:

It is not so easy for a beginner to distinguish between a good tutorial and a bad tutorial, especially if it has not been completed yet.
This is related to the content. ^

So I believe you when you say that it’s not good and/or outdated, I just had too many things that didn’t really work.

I’m trying to implement password_verify today when I am back at home.
You mean that hashing is not the best method, at least that’s what I read. I’ll see if I can find out about alternatives.
At least the progress is there and to be honest I only owe it to you in this topic :wink:

I also have the feeling that the actual game will ultimately be easier to develop than the part I’m currently working on.

1 Like

It isn’t easy for professionals either.

What you are attempting is NOT a beginner thing.

Look at how many data breaches happen every single day.

Those breaches happen in code developed by senior engineers.

Those breaches happen because of EXACTLY the kind of naive assumptions discussed in this thread, assumptions that lead to designs that simply break the moment someone tries something malicious.

^ ^ ^ This above statement is like saying “I want to learn how to make a Space Shuttle but for now I only know how to work using wood and nails, so I want to finish my first Space Shuttle using only wood and nails, THEN go onto learn how to make an actual functioning spaceship.” It isn’t likely that anything you learn doing it the wrong way is going to be useful, except perhaps general coding stuff, but that applies across the board.

Pay close attention to what people like @Bunny83 and others post in this thread. Security stuff is HARD. Really really really HARD. I know almost nothing about it and I know that I know nothing about it and I would never even consider attempting something with a login / password system, and I’ve been doing this for 40 years. What you contemplate is a MASSIVE undertaking to truly make it secure.

Use something like one of the existing game login systems you can sign up for. Don’t try and roll this stuff yourself.

1 Like