0

I want to login into my program via MySQL (Database).

string username = textBox1.Text.ToString();
int UserID = DB.runRead("SELECT id FROM users WHERE username='" + DB.Stripslash(username) + "'", null);
if (UserID > 0)
{
    String[] UserData = DB.runReadRow("SELECT id, username, password, salt, rank, verein FROM benutzer WHERE id=" + UserID.ToString());
    if(UserData[1].ToString().ToLower() == textBox2.Text.ToString().ToLower())
    {
        // logged in
        SetStatusText("logged in!! :)");
        SetProgressbarValue(100);
        ButtonActivity(0, false);
    }
    else
    {
        // Password wrong
        SetStatusText("wrong pw!!");
        SetProgressbarValue(50);
    }
}
else
{
    SetStatusText("unregistered!!");
    SetProgressbarValue(0);
}

I use this method on some other projects. Always working fine, but this time, the statement

if(UserID > 0)

and it's following code won't run, due to UserID having a value of 0, and I don't know why. Of course there is a user in the Database:

screenshot of DB results showing 1 User row with id=1

I already tried to use another database (new created), but the error stays, so I think I did something stupid within the code. If necessary I could post the code of the working one, but I see no difference.

int UserID = DB.runRead("SELECT id FROM benutzer WHERE username='" + DB.Stripslash(Connection.Username) + "'", null);
if (UserID > 0)
{
    string[] UserData = DB.runReadRow("SELECT id, username, password, salt, online, nickname, rank, firstlogin, bantime, anticheat FROM users WHERE id=" + UserID.ToString());
    //Password = hashMD5(hashMD5(getBlock(3).ToLower()) + hashMD5(UserData[1]));
    if (UserData[2].ToLower() == Password && UserData[9].ToString() == "1")
    {
        if (BanManager.isBlocked(UserID) == false /*&& RankManager.HasPermision(int.Parse(UserData[6]), "account.authorize")*/)
        {
            if (UserData[4].Equals("1"))
            {
                ReturnValue = LoginState.AlreadyLoggedIn;
                Connection.send(new PACKET_SERVER_LIST(PACKET_SERVER_LIST.errorCodes.AlreadyLoggedIn));
                //Log.WriteLine("Connection from " + Connection.IPAddress + " logged succesfull in as " + UserData[5] + " but the user is already online.");
                Log.WriteLine("- Login - A Connection was successfully approved");
                Log.WriteLine("- Login - IP Address: " + Connection.IPAddress + ",");
                Log.WriteLine("- Login - Nickname: " + UserData[5] + ",");
                Log.WriteLine("- Login - Status: The User is already logged in.");
                Console.ForegroundColor = ConsoleColor.DarkCyan; Console.WriteLine(new String('*', Console.WindowWidth)); Console.ForegroundColor = ConsoleColor.Gray;
            }

And so on. What's wrong?

Rhumborl
  • 16,349
  • 4
  • 39
  • 45
ToxicData
  • 3
  • 2
  • 2
    Please take more care when formatting your posts - the indentation pushed most of the code off the screen to start with. Use the preview to check that the post looks how you'd want it to look if you were trying to answer. – Jon Skeet Oct 22 '14 at 10:15
  • 2
    Next, *stop building SQL like that*. Use parameterized SQL. *Always*. And it looks like you're storing your passwords in plain text... which is another massive problem. – Jon Skeet Oct 22 '14 at 10:15
  • @JonSkeet I got it like this, I never used something else. – ToxicData Oct 22 '14 at 10:17
  • That's no excuse to *leave* it horribly insecure like that. Please read http://bobbytables.com – Jon Skeet Oct 22 '14 at 10:17
  • @JonSkeet: That site is down. Getting a message: BobbyTables.com is for sale (Bobby Tables) – Rohith Oct 22 '14 at 10:20
  • 1
    Then look at http://stackoverflow.com/questions/19326563/string-field-with-single-quotation-mark-is-causing-an-error-when-inserting-recor#19326598 or in an humorous way to http://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work – Steve Oct 22 '14 at 10:23
  • 1
    @Rohith: Whoops - I meant http://bobby-tables.com – Jon Skeet Oct 22 '14 at 10:23
  • A part from the Sql Injection problem, you should ask yourself how a user with ID = 0 has found its way to be stored in the database. If your code uses that if condition then NO USER should be stored with ID=0. There is no answer here, you should fix the logical problem when you insert new users in the database – Steve Oct 22 '14 at 10:31

1 Answers1

0

It depends on what's happening within runRead. I haven't done a lot of DB programming in C# but I'm not aware of that call being in any of the standard classes, nor in the MySQL stuff. I could be wrong in which case feel free to educate me, and I'll ditch this answer.

I did find a piece of code in my searches that looked like this:

public static int runRead(string Query, object Tick) {
    checkConnection();
    try {
        return Convert.ToInt32(new MySqlCommand(Query + " LIMIT 1", dbConnection).ExecuteScalar());
    } catch {
        return 0;
    }
}

and, if that's the one you're using, that might explain why you're getting back zero - there could be an exception happening that you catching and ignoring.

One way to find out is to remove the exception catch, or log a suitable message at the time it happens, assuming you can change the source.


The other thing you should look out for is if you're using a user name that doesn't exist in the database. Since the query will then return zero rows, I'm not entirely certain what it will try to do with the "first column in the first row" that ExecuteScalar() tries to get.

Community
  • 1
  • 1
paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953